Merge pull request oraios#225 from oraios/editing-improvements

Editing improvements

Author: MischaPanch

CHANGELOG.md

@@ -13,6 +13,8 @@ Status of the `main` branch. Changes prior to the next official version change w
 Fixes:
 * Fix `ExecuteShellCommandTool` and `GetCurrentConfigTool` hanging on Windows
 * Fix project activation by name via `--project` not working (was broken in previous release) 
+* Improve handling of indentation and newlines in symbolic editing tools
+* Fix that `insert_after_symbol` was failing for insertions at the end of a file that did not end with a newline
 
 # 2025-06-20
 

pyproject.toml

@@ -73,7 +73,7 @@ google = [
 Homepage = "https://github.com/oraios/serena"
 
 [tool.hatch.build.targets.wheel]
-packages = ["src/serena", "src/multilspy", "src/interprompt", "src/solidlsp"]
+packages = ["src/serena", "src/interprompt", "src/solidlsp"]
 
 [tool.black]
 line-length = 140
@@ -82,7 +82,7 @@ target-version = [
 ]
 exclude = '''
 /(
-    src/multilspy
+    src/solidlsp/language_servers/.*/static,
 )/
 '''
 
@@ -138,7 +138,7 @@ type-check = [
 target-version = "py311"
 line-length = 140
 exclude = [
-  "src/multilspy",
+    "src/solidlsp/language_servers/**/static",
 ]
 
 [tool.ruff.format]

pytest.ini

@@ -0,0 +1,2 @@
+[pytest]
+addopts = --snapshot-patch-pycharm-diff

src/serena/agent.py

@@ -1693,27 +1693,16 @@ def apply(
         r"""
         Replaces the body of the symbol with the given `name_path`.
 
-        Important:
-        You don't need to provide an adjusted indentation,
-        as the tool will automatically add the indentation of the original symbol to each line. For example,
-        for replacing a method in python, you can just write (using the standard python indentation):
-        body="def my_method_replacement(self, ...):\n    first_line\n    second_line...". So each line after the first line only has
-        an indentation of 4 (the indentation relative to the first character),
-        since the additional indentation will be added by the tool. Same for more deeply nested
-        cases. You always only need to write the relative indentation to the first character of the first line, and that
-        in turn should not have any indentation.
-        ALWAYS REMEMBER TO USE THE CORRECT INDENTATION IN THE BODY!
-
         :param name_path: for finding the symbol to replace, same logic as in the `find_symbol` tool.
         :param relative_path: the relative path to the file containing the symbol
-        :param body: the new symbol body.
-
+        :param body: the new symbol body. Important: Begin directly with the symbol definition and provide no
+            leading indentation for the first line (but do indent the rest of the body according to the context).
         """
         self.symbol_manager.replace_body(
             name_path,
             relative_file_path=relative_path,
             body=body,
-            use_same_indentation=True,
+            use_same_indentation=False,
         )
         return SUCCESS_RESULT
 
@@ -1733,17 +1722,12 @@ def apply(
         Inserts the given body/content after the end of the definition of the given symbol (via the symbol's location).
         A typical use case is to insert a new class, function, method, field or variable assignment.
 
-        :param name_path: for finding the symbol to insert after, same logic as in the `find_symbol` tool.
+        :param name_path: name path of the symbol after which to insert content (definitions in the `find_symbol` tool apply)
         :param relative_path: the relative path to the file containing the symbol
-        :param body: the body/content to be inserted. Important: the inserted code will automatically have the
-            same indentation as the symbol's body, so you do not need to provide any additional indentation.
+        :param body: the body/content to be inserted. The inserted code shall begin with the next line after
+            the symbol.
         """
-        self.symbol_manager.insert_after_symbol(
-            name_path,
-            relative_file_path=relative_path,
-            body=body,
-            use_same_indentation=True,
-        )
+        self.symbol_manager.insert_after_symbol(name_path, relative_file_path=relative_path, body=body, use_same_indentation=False)
         return SUCCESS_RESULT
 
 
@@ -1763,17 +1747,11 @@ def apply(
         A typical use case is to insert a new class, function, method, field or variable assignment.
         It also can be used to insert a new import statement before the first symbol in the file.
 
-        :param name_path: for finding the symbol to insert before, same logic as in the `find_symbol` tool.
+        :param name_path: name path of the symbol before which to insert content (definitions in the `find_symbol` tool apply)
         :param relative_path: the relative path to the file containing the symbol
-        :param body: the body/content to be inserted. Important: the inserted code will automatically have the
-            same indentation as the symbol's body, so you do not need to provide any additional indentation.
+        :param body: the body/content to be inserted before the line in which the referenced symbol is defined
         """
-        self.symbol_manager.insert_before_symbol(
-            name_path,
-            relative_file_path=relative_path,
-            body=body,
-            use_same_indentation=True,
-        )
+        self.symbol_manager.insert_before_symbol(name_path, relative_file_path=relative_path, body=body, use_same_indentation=False)
         return SUCCESS_RESULT
 
 

src/serena/resources/config/modes/editing.yml

@@ -23,8 +23,7 @@ prompt: |
   use `find_symbol` with the name path `Foo/__init__` and `include_body=True`. If you don't know yet which methods in `Foo` you need to read or edit,
   you can use `find_symbol` with the name path `Foo`, `include_body=False` and `depth=1` to get all (top-level) methods of `Foo` before proceeding
   to read the desired methods with `include_body=True`.
-  Note that you never need to add additional indentation, as all symbol editing tools will automatically add the indentation of the symbol that
-  you are replacing or inserting above or below. In particular, keep in mind the description of the `replace_symbol_body` tool. If you want to add some new code at the end of the file, you should
+  In particular, keep in mind the description of the `replace_symbol_body` tool. If you want to add some new code at the end of the file, you should
   use the `insert_after_symbol` tool with the last top-level symbol in the file. If you want to add an import, often a good strategy is to use
   `insert_before_symbol` with the first top-level symbol in the file.
   You can understand relationships between symbols by using the `find_referencing_symbols` tool. If not explicitly requested otherwise by a user,

src/serena/symbol.py

@@ -1,7 +1,7 @@
 import json
 import logging
 import os
-from collections.abc import Iterator, Sequence
+from collections.abc import Iterable, Iterator, Reversible, Sequence
 from contextlib import contextmanager
 from dataclasses import asdict, dataclass, field
 from difflib import SequenceMatcher
@@ -256,6 +256,13 @@ def kind(self) -> str:
     def symbol_kind(self) -> SymbolKind:
         return self.symbol_root["kind"]
 
+    def is_neighbouring_definition_separated_by_empty_line(self) -> bool:
+        """
+        :return: whether a symbol definition of this symbol's kind is usually separated from the
+            previous/next definition by at least one empty line.
+        """
+        return self.symbol_kind in (SymbolKind.Function, SymbolKind.Method, SymbolKind.Class, SymbolKind.Interface, SymbolKind.Struct)
+
     @property
     def relative_path(self) -> str | None:
         location = self.symbol_root.get("location")
@@ -689,26 +696,36 @@ def replace_body_at_location(self, location: SymbolLocation, body: str, *, use_s
             if start_pos is None or end_pos is None:
                 raise ValueError(f"Symbol at {location} does not have a defined body range.")
             start_line, start_col = start_pos["line"], start_pos["character"]
+
             if use_same_indentation:
                 indent = " " * start_col
                 body_lines = body.splitlines()
                 body = body_lines[0] + "\n" + "\n".join(indent + line for line in body_lines[1:])
 
-            # make sure body always ends with at least one newline
-            if not body.endswith("\n"):
-                body += "\n"
+            # make sure the replacement adds no additional newlines (before or after) - all newlines
+            # and whitespace before/after should remain the same, so we strip it entirely
+            body = body.strip()
+
             self._lang_server.delete_text_between_positions(location.relative_path, start_pos, end_pos)
             self._lang_server.insert_text_at_position(location.relative_path, start_line, start_col, body)
 
-    def insert_after_symbol(
-        self,
-        name_path: str,
-        relative_file_path: str,
-        body: str,
-        *,
-        use_same_indentation: bool = True,
-        at_new_line: bool = True,
-    ) -> None:
+    @staticmethod
+    def _count_leading_newlines(text: Iterable) -> int:
+        cnt = 0
+        for c in text:
+            if c == "\n":
+                cnt += 1
+            elif c == "\r":
+                continue
+            else:
+                break
+        return cnt
+
+    @classmethod
+    def _count_trailing_newlines(cls, text: Reversible) -> int:
+        return cls._count_leading_newlines(reversed(text))
+
+    def insert_after_symbol(self, name_path: str, relative_file_path: str, body: str, *, use_same_indentation: bool = True) -> None:
         """
         Inserts content after the symbol with the given name in the given file.
         """
@@ -722,13 +739,9 @@ def insert_after_symbol(
                 f"Found symbols at locations: \n" + json.dumps([s.location.to_dict() for s in symbol_candidates], indent=2)
             )
         symbol = symbol_candidates[-1]
-        return self.insert_after_symbol_at_location(
-            symbol.location, body, at_new_line=at_new_line, use_same_indentation=use_same_indentation
-        )
+        return self.insert_after_symbol_at_location(symbol.location, body, use_same_indentation=use_same_indentation)
 
-    def insert_after_symbol_at_location(
-        self, location: SymbolLocation, body: str, *, at_new_line: bool = True, use_same_indentation: bool = True
-    ) -> None:
+    def insert_after_symbol_at_location(self, location: SymbolLocation, body: str, *, use_same_indentation: bool = True) -> None:
         """
         Appends content after the given symbol
 
@@ -750,12 +763,23 @@ def insert_after_symbol_at_location(
         if pos is None:
             raise ValueError(f"Symbol at {location} does not have a defined end position.")
 
-        line, col = pos["line"], pos["character"]
-        if at_new_line:
-            line += 1
-            col = 0
-            if not body.startswith("\n"):
-                body = "\n" + body
+        # start at the beginning of the next line
+        col = 0
+        line = pos["line"] + 1
+        # make sure a suitable number of leading empty lines is used (at least 0/1 depending on the symbol type,
+        # otherwise as many as the caller wanted to insert)
+        original_leading_newlines = self._count_leading_newlines(body)
+        body = body.lstrip("\r\n")
+        min_empty_lines = 0
+        if symbol.is_neighbouring_definition_separated_by_empty_line():
+            min_empty_lines = 1
+        num_leading_empty_lines = max(min_empty_lines, original_leading_newlines)
+        if num_leading_empty_lines:
+            body = ("\n" * num_leading_empty_lines) + body
+        # make sure the one line break succeeding the original symbol, which we repurposed as prefix via
+        # `line += 1`, is replaced
+        body = body.rstrip("\r\n") + "\n"
+
         if use_same_indentation:
             symbol_start_pos = symbol.body_start_position
             assert symbol_start_pos is not None, f"Symbol at {location=} does not have a defined start position."
@@ -778,20 +802,11 @@ def insert_after_symbol_at_location(
             # > test test
             # > second line
             # > dataclass_instance.status = "active"  # Reassign dataclass field
-            col = 0
 
         with self._edited_symbol_location(location):
             self._lang_server.insert_text_at_position(location.relative_path, line=line, column=col, text_to_be_inserted=body)
 
-    def insert_before_symbol(
-        self,
-        name_path: str,
-        relative_file_path: str,
-        body: str,
-        *,
-        at_new_line: bool = True,
-        use_same_indentation: bool = True,
-    ) -> None:
+    def insert_before_symbol(self, name_path: str, relative_file_path: str, body: str, *, use_same_indentation: bool = True) -> None:
         """
         Inserts content before the symbol with the given name in the given file.
         """
@@ -805,11 +820,9 @@ def insert_before_symbol(
                 f"Found symbols at locations: \n" + json.dumps([s.location.to_dict() for s in symbol_candidates], indent=2)
             )
         symbol = symbol_candidates[0]
-        self.insert_before_symbol_at_location(symbol.location, body, at_new_line=at_new_line, use_same_indentation=use_same_indentation)
+        self.insert_before_symbol_at_location(symbol.location, body, use_same_indentation=use_same_indentation)
 
-    def insert_before_symbol_at_location(
-        self, location: SymbolLocation, body: str, *, at_new_line: bool = True, use_same_indentation: bool = True
-    ) -> None:
+    def insert_before_symbol_at_location(self, location: SymbolLocation, body: str, *, use_same_indentation: bool = True) -> None:
         """
         Inserts content before the given symbol
 
@@ -820,18 +833,28 @@ def insert_before_symbol_at_location(
             symbol_start_pos = symbol.body_start_position
             if symbol_start_pos is None:
                 raise ValueError(f"Symbol at {location} does not have a defined start position.")
-            line = symbol_start_pos["line"]
-            col = symbol_start_pos["character"]
+
             if use_same_indentation:
-                indent = " " * (col)
+                indent = " " * (symbol_start_pos["character"])
                 body = "\n".join(indent + line for line in body.splitlines())
 
-            # similar problems as in insert_after_symbol_at_location, see comment there
-            if at_new_line:
-                col = 0
-                line -= 1
-                if not body.endswith("\n"):
-                    body += "\n"
+            # insert position is the start of line where the symbol is defined
+            line = symbol_start_pos["line"]
+            col = 0
+
+            original_trailing_empty_lines = self._count_trailing_newlines(body) - 1
+
+            # ensure eol is present at end
+            body = body.rstrip() + "\n"
+
+            # add suitable number of trailing empty lines after the body (at least 0/1 depending on the symbol type,
+            # otherwise as many as the caller wanted to insert)
+            min_trailing_empty_lines = 0
+            if symbol.is_neighbouring_definition_separated_by_empty_line():
+                min_trailing_empty_lines = 1
+            num_trailing_newlines = max(min_trailing_empty_lines, original_trailing_empty_lines)
+            body += "\n" * num_trailing_newlines
+
             assert location.relative_path is not None
 
             self._lang_server.insert_text_at_position(location.relative_path, line=line, column=col, text_to_be_inserted=body)

src/solidlsp/language_servers/clangd_language_server/clangd_language_server.py

@@ -10,11 +10,11 @@
 import threading
 
 from solidlsp.ls import SolidLanguageServer
-from solidlsp.lsp_protocol_handler.lsp_types import InitializeParams
-from solidlsp.lsp_protocol_handler.server import ProcessLaunchInfo
 from solidlsp.ls_config import LanguageServerConfig
 from solidlsp.ls_logger import LanguageServerLogger
 from solidlsp.ls_utils import FileUtils, PlatformUtils
+from solidlsp.lsp_protocol_handler.lsp_types import InitializeParams
+from solidlsp.lsp_protocol_handler.server import ProcessLaunchInfo
 
 
 class ClangdLanguageServer(SolidLanguageServer):

src/solidlsp/language_servers/dart_language_server/dart_language_server.py

@@ -5,9 +5,9 @@
 import stat
 
 from solidlsp.ls import SolidLanguageServer
-from solidlsp.lsp_protocol_handler.server import ProcessLaunchInfo
 from solidlsp.ls_logger import LanguageServerLogger
 from solidlsp.ls_utils import FileUtils, PlatformUtils
+from solidlsp.lsp_protocol_handler.server import ProcessLaunchInfo
 
 
 class DartLanguageServer(SolidLanguageServer):

src/solidlsp/language_servers/eclipse_jdtls/eclipse_jdtls.py

@@ -16,12 +16,12 @@
 from overrides import override
 
 from solidlsp.ls import SolidLanguageServer
-from solidlsp.lsp_protocol_handler.lsp_types import InitializeParams
-from solidlsp.lsp_protocol_handler.server import ProcessLaunchInfo
 from solidlsp.ls_config import LanguageServerConfig
 from solidlsp.ls_logger import LanguageServerLogger
-from solidlsp.settings import SolidLSPSettings
 from solidlsp.ls_utils import FileUtils, PlatformUtils
+from solidlsp.lsp_protocol_handler.lsp_types import InitializeParams
+from solidlsp.lsp_protocol_handler.server import ProcessLaunchInfo
+from solidlsp.settings import SolidLSPSettings
 
 
 @dataclasses.dataclass

src/solidlsp/language_servers/gopls/gopls.py

@@ -8,10 +8,10 @@
 from overrides import override
 
 from solidlsp.ls import SolidLanguageServer
-from solidlsp.lsp_protocol_handler.lsp_types import InitializeParams
-from solidlsp.lsp_protocol_handler.server import ProcessLaunchInfo
 from solidlsp.ls_config import LanguageServerConfig
 from solidlsp.ls_logger import LanguageServerLogger
+from solidlsp.lsp_protocol_handler.lsp_types import InitializeParams
+from solidlsp.lsp_protocol_handler.server import ProcessLaunchInfo
 
 
 class Gopls(SolidLanguageServer):