Merge branch 'develop'

Author: tumf

CHANGELOG.md

@@ -1,5 +1,34 @@
 # Changelog
 
+## [1.1.0] - 2024-12-23
+
+### Added
+
+- New text file manipulation operations:
+  - `insert_text_file_contents`: Insert content at specific positions
+  - `create_text_file`: Create new text files
+  - `append_text_file_contents`: Append content to existing files
+  - `delete_text_file_contents`: Delete specified ranges of text
+  - `patch_text_file_contents`: Apply multiple patches to text files
+- Enhanced error messages with useful suggestions for alternative editing methods
+
+### Changed
+
+- Unified parameter naming: renamed 'path' to 'file_path' across all APIs
+- Improved handler organization by moving them to separate directory
+- Made 'end' parameter required when not in append mode
+- Enhanced validation for required parameters and file path checks
+- Removed 'edit_text_file_contents' tool in favor of more specific operations
+- Improved JSON serialization for handler responses
+
+### Fixed
+
+- Delete operation now uses dedicated deletion method instead of empty content replacement
+- Improved range validation in delete operations
+- Enhanced error handling across all operations
+- Removed file hash from error responses for better clarity
+- Fixed concurrency control with proper hash validation
+
 ## [1.0.2] - 2024-12-22
 
 ### Fixed

src/mcp_text_editor/text_editor.py

@@ -11,32 +11,6 @@
 logger = logging.getLogger(__name__)
 
 
-def _create_error_response(
-    error_message: str,
-    content_hash: Optional[str] = None,
-    file_path: Optional[str] = None,
-) -> Dict[str, Any]:
-    """Create a standardized error response.
-
-    Args:
-        error_message (str): The error message to include
-        content_hash (Optional[str], optional): Hash of the current content if available
-        file_path (Optional[str], optional): File path to use as dictionary key
-
-    Returns:
-        Dict[str, Any]: Standardized error response structure
-    """
-    error_response = {
-        "result": "error",
-        "reason": error_message,
-        "hash": content_hash,
-    }
-
-    if file_path:
-        return {file_path: error_response}
-    return error_response
-
-
 class TextEditor:
     """Handles text file operations with security checks and conflict detection."""
 
@@ -45,6 +19,44 @@ def __init__(self):
         self._validate_environment()
         self.service = TextEditorService()
 
+    def create_error_response(
+        self,
+        error_message: str,
+        content_hash: Optional[str] = None,
+        file_path: Optional[str] = None,
+        suggestion: Optional[str] = None,
+        hint: Optional[str] = None,
+    ) -> Dict[str, Any]:
+        """Create a standardized error response.
+
+        Args:
+            error_message (str): The error message to include
+            content_hash (Optional[str], optional): Hash of the current content if available
+            file_path (Optional[str], optional): File path to use as dictionary key
+            suggestion (Optional[str], optional): Suggested operation type
+            hint (Optional[str], optional): Hint message for users
+
+        Returns:
+            Dict[str, Any]: Standardized error response structure
+        """
+        error_response = {
+            "result": "error",
+            "reason": error_message,
+            "file_hash": content_hash,
+        }
+
+        # Add fields if provided
+        if content_hash is not None:
+            error_response["file_hash"] = content_hash
+        if suggestion:
+            error_response["suggestion"] = suggestion
+        if hint:
+            error_response["hint"] = hint
+
+        if file_path:
+            return {file_path: error_response}
+        return error_response
+
     def _validate_environment(self) -> None:
         """
         Validate environment variables and setup.
@@ -241,20 +253,22 @@ async def edit_file_contents(
         try:
             if not os.path.exists(file_path):
                 if expected_hash not in ["", None]:  # Allow null hash
-                    return {
-                        "result": "error",
-                        "reason": "File not found and non-empty hash provided",
-                    }
+                    return self.create_error_response(
+                        "File not found and non-empty hash provided",
+                        suggestion="append",
+                        hint="For new files, please consider using append_text_file_contents",
+                    )
                 # Create parent directories if they don't exist
                 parent_dir = os.path.dirname(file_path)
                 if parent_dir:
                     try:
                         os.makedirs(parent_dir, exist_ok=True)
                     except OSError as e:
-                        return {
-                            "result": "error",
-                            "reason": f"Failed to create directory: {str(e)}",
-                        }
+                        return self.create_error_response(
+                            f"Failed to create directory: {str(e)}",
+                            suggestion="patch",
+                            hint="Please check file permissions and try again",
+                        )
                 # Initialize empty state for new file
                 current_content = ""
                 current_hash = ""
@@ -277,17 +291,21 @@ async def edit_file_contents(
                     current_hash = ""
                     lines = []
                 elif current_content and expected_hash == "":
-                    return {
-                        "result": "error",
-                        "reason": "Unexpected error - Cannot treat existing file as new",
-                    }
+                    return self.create_error_response(
+                        "Unexpected error - Cannot treat existing file as new",
+                    )
                 elif current_hash != expected_hash:
-                    return {
-                        "result": "error",
-                        "reason": "FileHash mismatch - Please use get_text_file_contents tool to get current content and hashes, then retry with the updated hashes.",
-                    }
+                    suggestion = "patch"
+                    hint = "Please use get_text_file_contents tool to get the current content and hash"
+
+                    return self.create_error_response(
+                        "FileHash mismatch - Please use get_text_file_contents tool to get current content and hashes, then retry with the updated hashes.",
+                        suggestion=suggestion,
+                        hint=hint,
+                    )
                 else:
                     lines = current_content.splitlines(keepends=True)
+                    lines = current_content.splitlines(keepends=True)
 
             # Convert patches to EditPatch objects
             patch_objects = [EditPatch.model_validate(p) for p in patches]
@@ -314,10 +332,11 @@ async def edit_file_contents(
                     if (start1 <= end2 and end1 >= start2) or (
                         start2 <= end1 and end2 >= start1
                     ):
-                        return {
-                            "result": "error",
-                            "reason": "Overlapping patches detected",
-                        }
+                        return self.create_error_response(
+                            "Overlapping patches detected",
+                            suggestion="patch",
+                            hint="Please ensure your patches do not overlap",
+                        )
 
             # Apply patches
             for patch in sorted_patches:
@@ -364,7 +383,6 @@ async def edit_file_contents(
                     # New file or empty file - treat as insertion
                     is_insertion = True
                 elif start_zero >= len(lines):
-                    # Append mode - start exceeds total lines
                     is_insertion = True
                 else:
                     # For modification mode, check the range_hash
@@ -386,6 +404,8 @@ async def edit_file_contents(
                             return {
                                 "result": "error",
                                 "reason": "Content range hash mismatch - Please use get_text_file_contents tool with the same start and end to get current content and hashes, then retry with the updated hashes.",
+                                "suggestion": "get",
+                                "hint": "Please run get_text_file_contents first to get current content and hashes",
                             }
 
                 # Prepare new content
@@ -404,18 +424,18 @@ async def edit_file_contents(
                     }
 
                 # Set suggestions for alternative tools
-                suggestion = None
-                hint = None
+                suggestion_text: Optional[str] = None
+                hint_text: Optional[str] = None
                 if not os.path.exists(file_path) or not current_content:
-                    suggestion = "append"
-                    hint = "For new or empty files, please consider using append_text_file_contents instead"
+                    suggestion_text = "append"
+                    hint_text = "For new or empty files, please consider using append_text_file_contents instead"
                 elif is_insertion:
                     if start_zero >= len(lines):
-                        suggestion = "append"
-                        hint = "For adding content at the end of file, please consider using append_text_file_contents instead"
+                        suggestion_text = "append"
+                        hint_text = "For adding content at the end of file, please consider using append_text_file_contents instead"
                     else:
-                        suggestion = "insert"
-                        hint = "For inserting content within file, please consider using insert_text_file_contents instead"
+                        suggestion_text = "insert"
+                        hint_text = "For inserting content within file, please consider using insert_text_file_contents instead"
 
                 # Prepare the content
                 new_content = contents if contents.endswith("\n") else contents + "\n"
@@ -441,24 +461,32 @@ async def edit_file_contents(
                 "result": "ok",
                 "file_hash": new_hash,
                 "reason": None,
-                "suggestion": suggestion,
-                "hint": hint,
+                "suggestion": suggestion_text,
+                "hint": hint_text,
             }
 
         except FileNotFoundError:
-            return {"result": "error", "reason": f"File not found: {file_path}"}
+            return self.create_error_response(
+                f"File not found: {file_path}",
+                suggestion="append",
+                hint="For new files, please use append_text_file_contents",
+            )
         except (IOError, UnicodeError, PermissionError) as e:
-            return {"result": "error", "reason": f"Error editing file: {str(e)}"}
+            return self.create_error_response(
+                f"Error editing file: {str(e)}",
+                suggestion="patch",
+                hint="Please check file permissions and try again",
+            )
         except Exception as e:
             import traceback
 
             logger.error(f"Error: {str(e)}")
             logger.error(f"Traceback:\n{traceback.format_exc()}")
-            return {
-                "result": "error",
-                "reason": "Unexpected error occurred",
-                "file_hash": None,
-            }
+            return self.create_error_response(
+                "Unexpected error occurred",
+                suggestion="patch",
+                hint="Please try again or report the issue if it persists",
+            )
 
     async def insert_text_file_contents(
         self,

tests/test_create_error_response.py

@@ -0,0 +1,56 @@
+"""Tests for error response creation and hint/suggestion functionality."""
+
+import pytest
+
+from mcp_text_editor.text_editor import TextEditor
+
+
+@pytest.fixture
+def editor():
+    """Create TextEditor instance."""
+    return TextEditor()
+
+
+def test_create_error_response_basic(editor):
+    """Test basic error response without hint/suggestion."""
+    response = editor.create_error_response("Test error")
+    assert response["result"] == "error"
+    assert response["reason"] == "Test error"
+    assert response["file_hash"] is None
+    assert "hint" not in response
+    assert "suggestion" not in response
+
+
+def test_create_error_response_with_hint_suggestion(editor):
+    """Test error response with hint and suggestion."""
+    response = editor.create_error_response(
+        "Test error", suggestion="append", hint="Please use append_text_file_contents"
+    )
+    assert response["result"] == "error"
+    assert response["reason"] == "Test error"
+    assert response["suggestion"] == "append"
+    assert response["hint"] == "Please use append_text_file_contents"
+
+
+def test_create_error_response_with_file_path(editor):
+    """Test error response with file path."""
+    response = editor.create_error_response(
+        "Test error",
+        file_path="/test/file.txt",
+        suggestion="patch",
+        hint="Please try again",
+    )
+    assert "/test/file.txt" in response
+    assert response["/test/file.txt"]["result"] == "error"
+    assert response["/test/file.txt"]["reason"] == "Test error"
+    assert response["/test/file.txt"]["suggestion"] == "patch"
+    assert response["/test/file.txt"]["hint"] == "Please try again"
+
+
+def test_create_error_response_with_hash(editor):
+    """Test error response with content hash."""
+    test_hash = "test_hash_value"
+    response = editor.create_error_response("Test error", content_hash=test_hash)
+    assert response["result"] == "error"
+    assert response["reason"] == "Test error"
+    assert response["file_hash"] == test_hash