From 9208a3ae877d4371ff3df6d6c5c2df8019155a8a Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Tue, 14 Jul 2026 16:25:54 +0200 Subject: [PATCH 1/5] Improve line/column computations, adhering to LSP standard * Add helper class TextStepper to facilitate computations * Reimplement TextUtils methods based on TextStepper * Adhere to LSP definition of newlines being defined by either "\n", "\r\n" or "\r" on its own. Previously, only "\n" was considered. * Add util method TextUtils.split_lines, allowing usages of Python's splitlines to be replaced --- CHANGELOG.md | 1 + src/solidlsp/ls_utils.py | 206 ++++++++++++++++++++++++++----- test/solidlsp/test_text_utils.py | 32 +++++ 3 files changed, 205 insertions(+), 34 deletions(-) create mode 100644 test/solidlsp/test_text_utils.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 4952c3bf..92adb837 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,7 @@ Status of the `main` branch. Changes prior to the next official version change w - During project creation, language composition percentages are now computed relative to the total number of recognised source files instead of all files, i.e. unrecognised files are ignored in the percentage computation. + - Fix: Use LSP-compliant line splitting ("\n", "\r\n" and "\r" can define line breaks) * CLI: - Fix `--project-from-cwd` hijacking git worktrees nested under a Serena project. `find_project_root` diff --git a/src/solidlsp/ls_utils.py b/src/solidlsp/ls_utils.py index 0f47a3e7..321827bf 100644 --- a/src/solidlsp/ls_utils.py +++ b/src/solidlsp/ls_utils.py @@ -31,6 +31,114 @@ class InvalidTextLocationError(Exception): pass +class TextStepper: + def __init__(self, chars: str): + self._chars = chars + self._len = len(chars) + self.line = 0 + """ + the current 0-based line index + """ + self.col = 0 + """ + the current 0-based column index + """ + self.idx = 0 + """ + the current 0-based index in the full text. + The index specifies the next character to be processed, i.e. if this is + the length of the text, then the end of the text has been reached. + It specifies a location in the same way as a cursor insertion position: + cursor at the very beginning (idx=0) means insert before the first character. + """ + self._is_newline = False + """ + whether the last step was a line break + """ + self._prev_line_start_idx = 0 + """ + start index of the last fully processed line (inclusive) + """ + self._prev_line_end_idx = 0 + """ + end of the last fully processed line (exclusive), excluding newline characters + """ + self._line_start_idx = 0 + """ + start of the current, not yet completed line (inclusive) + """ + + def _get_char(self, idx: int) -> str | None: + if idx < 0 or idx >= self._len: + return None + return self._chars[idx] + + def step(self) -> bool: + """ + Processes the next character/newline sequence in the text + + :return: True if processing was possible, False if the end of the text was reached + """ + if self.idx >= self._len: + return False + + # process next character/newline sequence + # Note: LSP defines that a newline is given by either "\n", "\r\n", or "\r" on its own + # Reference: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.18/specification/#textDocuments + idx_before = self.idx + c = self._chars[self.idx] + self.idx += 1 + is_newline = False + if c == "\n": + is_newline = True + elif c == "\r": + is_newline = True + if self._get_char(self.idx) == "\n": + self.idx += 1 # skip an additional character + + self._is_newline = is_newline + if is_newline: + self.line += 1 + self.col = 0 + self._prev_line_end_idx = idx_before + self._prev_line_start_idx = self._line_start_idx + self._line_start_idx = self.idx + else: + self.col += 1 + return True + + def process_all(self): + """ + Processes all characters in the text, updating the line and column numbers accordingly. + """ + while self.step(): + pass + + def _get_last_line(self, with_end: bool) -> str: + """ + Returns the last line processed, optionally including the newline character(s) at the end + """ + start_idx = self._prev_line_start_idx + end_idx = self._prev_line_end_idx if not with_end else self._line_start_idx + return self._chars[start_idx:end_idx] + + def process_all_gather_lines(self, with_ends: bool) -> list[str]: + """ + Processes all characters in the text and returns a list of lines + + :param with_ends: whether to include the newline character(s) at the end of each line + :return: the list of lines + """ + lines = [] + while self.step(): + if self._is_newline: + lines.append(self._get_last_line(with_end=with_ends)) + # add the last line (which was not followed by a newline), even if empty + last_line = self._chars[self._line_start_idx :] + lines.append(last_line) + return lines + + class TextUtils: """ Utilities for text operations. @@ -39,54 +147,68 @@ class TextUtils: @staticmethod def get_line_col_from_index(text: str, index: int) -> tuple[int, int]: """ - Returns the zero-indexed line and column number of the given index in the given text + :param text: the text in which the index is to be located + :param index: the 0-based index in the text + :return: a tuple (0-based line number, 0-based column number) corresponding to the index in the text """ - l = 0 - c = 0 - idx = 0 - while idx < index: - if text[idx] == "\n": - l += 1 - c = 0 - else: - c += 1 - idx += 1 + text_stepper = TextStepper(text) + while text_stepper.idx < index: + if not text_stepper.step(): + raise InvalidTextLocationError + return text_stepper.line, text_stepper.col - return l, c + @classmethod + def get_line_from_index(cls, text: str, index: int) -> int: + """ + :param text: the text in which the index is to be located + :param index: the 0-based index in the text + :return: the 0-based line number corresponding to the index in the text + """ + return cls.get_line_col_from_index(text, index)[0] @staticmethod def get_index_from_line_col(text: str, line: int, col: int) -> int: """ - Returns the index of the given zero-indexed line and column number in the given text + :param text: the text in which the coordinates are to be located + :param line: the 0-based line number + :param col: the 0-based column number + :return: the corresponding 0-based index in the text """ - idx = 0 - while line > 0: - if idx >= len(text): + text_stepper = TextStepper(text) + while text_stepper.line < line: + if not text_stepper.step(): raise InvalidTextLocationError - if text[idx] == "\n": - line -= 1 - idx += 1 - idx += col + idx = text_stepper.idx + col return idx @staticmethod def _get_updated_position_from_line_and_column_and_edit(l: int, c: int, text_to_be_inserted: str) -> tuple[int, int]: """ - Utility function to get the position of the cursor after inserting text at a given line and column. + :param l: the 0-based line number before the edit + :param c: the 0-based column number before the edit + :param text_to_be_inserted: the text that was inserted at the given position + :return: the updated 0-based line and column numbers after the edit (end of insertion) """ - num_newlines_in_gen_text = text_to_be_inserted.count("\n") - if num_newlines_in_gen_text > 0: - l += num_newlines_in_gen_text - c = len(text_to_be_inserted.split("\n")[-1]) + text_stepper = TextStepper(text_to_be_inserted) + text_stepper.process_all() + if text_stepper.line > 0: + l += text_stepper.line + c = text_stepper.col else: - c += len(text_to_be_inserted) - return (l, c) + c += text_stepper.col + return l, c @staticmethod def delete_text_between_positions(text: str, start_line: int, start_col: int, end_line: int, end_col: int) -> tuple[str, str]: """ Deletes the text between the given start and end positions. - Returns the modified text and the deleted text. + + :param text: the original text + :param start_line: the 0-based line number of the start position + :param start_col: the 0-based column number of the start position + :param end_line: the 0-based line number of the end position + :param end_col: the 0-based column number of the end position + :return: a tuple containing the modified text and the deleted text """ del_start_idx = TextUtils.get_index_from_line_col(text, start_line, start_col) del_end_idx = TextUtils.get_index_from_line_col(text, end_line, end_col) @@ -98,13 +220,21 @@ class TextUtils: @staticmethod def insert_text_at_position(text: str, line: int, col: int, text_to_be_inserted: str) -> tuple[str, int, int]: """ - Inserts the given text at the given line and column. - Returns the modified text and the new line and column. + Inserts the given text at the given position and returns the + + :param text: the original text + :param line: the 0-based line number where the text should be inserted + :param col: the 0-based column number where the text should be inserted + :param text_to_be_inserted: the text to be inserted + :return: a tuple containing the modified text, the updated line number, and the updated column number + (position after the inserted text) """ try: change_index = TextUtils.get_index_from_line_col(text, line, col) except InvalidTextLocationError: - num_lines_in_text = text.count("\n") + 1 + text_stepper = TextStepper(text) + text_stepper.process_all() + num_lines_in_text = text_stepper.line + 1 max_line = num_lines_in_text - 1 if line == max_line + 1 and col == 0: # trying to insert at new line after full text # insert at end, adding missing newline @@ -125,14 +255,22 @@ class TextUtils: end_idx = TextUtils.get_index_from_line_col(text, end_line, end_col) return text[start_idx:end_idx] - @staticmethod - def get_text_in_lines_range(text: str, start_line: int, end_line: int) -> str: + @classmethod + def get_text_in_lines_range(cls, text: str, start_line: int, end_line: int) -> str: """ Returns the text encompassed by the given start and end lines (inclusive). """ - lines = text.splitlines(keepends=True) + lines = cls.split_lines(text, with_ends=True) return "".join(lines[start_line : end_line + 1]) + @staticmethod + def split_lines(text: str, with_ends: bool = False) -> list[str]: + """ + Splits the given text into lines, optionally including the newline character(s) at the end of each line. + """ + text_stepper = TextStepper(text) + return text_stepper.process_all_gather_lines(with_ends=with_ends) + class PathUtils: """ diff --git a/test/solidlsp/test_text_utils.py b/test/solidlsp/test_text_utils.py new file mode 100644 index 00000000..fa937242 --- /dev/null +++ b/test/solidlsp/test_text_utils.py @@ -0,0 +1,32 @@ +from solidlsp.ls_utils import TextUtils + + +class TestTextUtils: + LINE = "012" + TEXT = LINE + "\n" + LINE + "\r\n" + LINE + "\r" + LINE + + def test_split_lines(self): + lines = TextUtils.split_lines(self.TEXT, with_ends=False) + assert len(lines) == 4 + for line in lines: + assert line == self.LINE + + def test_split_lines_with_ends(self): + lines = TextUtils.split_lines(self.TEXT, with_ends=True) + assert len(lines) == 4 + for i, line in enumerate(lines): + assert line[: len(self.LINE)] == self.LINE + for i, ending in enumerate(["\n", "\r\n", "\r", ""]): + assert lines[i][len(self.LINE) :] == ending + + def test_line_col_from_idx(self): + assert TextUtils.get_line_col_from_index(self.TEXT, 0) == (0, 0) + assert TextUtils.get_line_col_from_index(self.TEXT, 1) == (0, 1) + assert TextUtils.get_line_col_from_index(self.TEXT, 3 + 1 + 1) == (1, 1) + assert TextUtils.get_line_col_from_index(self.TEXT, 3 + 1 + 3 + 2 + 1) == (2, 1) + + def test_idx_from_line_col(self): + assert TextUtils.get_index_from_line_col(self.TEXT, 0, 0) == 0 + assert TextUtils.get_index_from_line_col(self.TEXT, 0, 1) == 1 + assert TextUtils.get_index_from_line_col(self.TEXT, 1, 1) == 3 + 1 + 1 + assert TextUtils.get_index_from_line_col(self.TEXT, 2, 1) == 3 + 1 + 3 + 2 + 1 From 97c9b8c05de975b5bb064d5102667850fcfa711a Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Tue, 14 Jul 2026 16:39:59 +0200 Subject: [PATCH 2/5] Apply TextUtils, ensuring consistent line numbering * in search_text * in MatchedConsecutiveLines.from_file_contents --- CHANGELOG.md | 4 +++- src/serena/util/text_utils.py | 8 ++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 92adb837..6c0005ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,7 +33,9 @@ Status of the `main` branch. Changes prior to the next official version change w - During project creation, language composition percentages are now computed relative to the total number of recognised source files instead of all files, i.e. unrecognised files are ignored in the percentage computation. - - Fix: Use LSP-compliant line splitting ("\n", "\r\n" and "\r" can define line breaks) + - Fix: Use LSP-compliant line splitting ("\n", "\r\n" and "\r" can define line breaks); previously only "\n" considered + - Fix: Apply consistent line splitting across tools, uniformly applying the LSP splitting semantics; + affects `search_text` used by `search_for_pattern` tool (reported in #1684) * CLI: - Fix `--project-from-cwd` hijacking git worktrees nested under a Serena project. `find_project_root` diff --git a/src/serena/util/text_utils.py b/src/serena/util/text_utils.py index 992c2437..8deb9856 100644 --- a/src/serena/util/text_utils.py +++ b/src/serena/util/text_utils.py @@ -100,7 +100,7 @@ class MatchedConsecutiveLines: def from_file_contents( cls, file_contents: str, line: int, context_lines_before: int = 0, context_lines_after: int = 0, source_file_path: str | None = None ) -> Self: - line_contents = file_contents.split("\n") + line_contents = TextUtils.split_lines(file_contents) start_lineno = max(0, line - context_lines_before) end_lineno = min(len(line_contents) - 1, line + context_lines_after) text_lines: list[TextLine] = [] @@ -169,7 +169,7 @@ def search_text( raise ValueError("Pass either content or source_file_path") matches = [] - lines = content.splitlines() + lines = TextUtils.split_lines(content) total_lines = len(lines) # Convert pattern to a compiled regex if it's a string @@ -185,8 +185,8 @@ def search_text( end_pos = match.end() # Find the line numbers for the start and end positions - start_line_num = content[:start_pos].count("\n") - end_line_num = content[:end_pos].count("\n") + start_line_num = TextUtils.get_line_from_index(content, start_pos) + end_line_num = TextUtils.get_line_from_index(content, end_pos) # Calculate the range of lines to include in the context context_start = max(0, start_line_num - context_lines_before) From d4e99dc00309b6489b422295844b5a104a5efa0e Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Tue, 14 Jul 2026 23:09:49 +0200 Subject: [PATCH 3/5] Improve performance of TextStepper, stepping line by line instead of character by character, delegating the scan for line breaks to native string search (~15-100x speedup for a full pass over typical source files) --- src/solidlsp/ls_utils.py | 100 +++++++++++++++++++------------ test/solidlsp/test_text_utils.py | 2 + 2 files changed, 65 insertions(+), 37 deletions(-) diff --git a/src/solidlsp/ls_utils.py b/src/solidlsp/ls_utils.py index 321827bf..0f8ef140 100644 --- a/src/solidlsp/ls_utils.py +++ b/src/solidlsp/ls_utils.py @@ -51,19 +51,19 @@ class TextStepper: It specifies a location in the same way as a cursor insertion position: cursor at the very beginning (idx=0) means insert before the first character. """ - self._is_newline = False + self.is_newline = False """ - whether the last step was a line break + whether the last step processed a full line ending in a line break """ - self._prev_line_start_idx = 0 + self.prev_line_start_idx = 0 """ start index of the last fully processed line (inclusive) """ - self._prev_line_end_idx = 0 + self.prev_line_end_idx = 0 """ end of the last fully processed line (exclusive), excluding newline characters """ - self._line_start_idx = 0 + self.line_start_idx = 0 """ start of the current, not yet completed line (inclusive) """ @@ -73,53 +73,61 @@ class TextStepper: return None return self._chars[idx] - def step(self) -> bool: + def step_line(self) -> bool: """ - Processes the next character/newline sequence in the text + Processes the next line in the text, advancing past the next newline sequence or, + if no further newline is present, to the end of the text. - :return: True if processing was possible, False if the end of the text was reached + :return: True if processing was possible, False if the end of the text had already been reached + (idx not advanced) """ if self.idx >= self._len: return False - # process next character/newline sequence + # find the next newline sequence # Note: LSP defines that a newline is given by either "\n", "\r\n", or "\r" on its own # Reference: https://microsoft.github.io/language-server-protocol/specifications/lsp/3.18/specification/#textDocuments - idx_before = self.idx - c = self._chars[self.idx] - self.idx += 1 - is_newline = False - if c == "\n": - is_newline = True - elif c == "\r": - is_newline = True - if self._get_char(self.idx) == "\n": - self.idx += 1 # skip an additional character + # The search for "\r" is bounded by the position of the next "\n". + lf_idx = self._chars.find("\n", self.idx) + cr_idx = self._chars.find("\r", self.idx, lf_idx if lf_idx != -1 else self._len) + if cr_idx != -1: + newline_start_idx = cr_idx + newline_end_idx = cr_idx + 2 if self._get_char(cr_idx + 1) == "\n" else cr_idx + 1 + elif lf_idx != -1: + newline_start_idx = lf_idx + newline_end_idx = lf_idx + 1 + else: + newline_start_idx = None + newline_end_idx = None - self._is_newline = is_newline - if is_newline: + # advance past the newline sequence or, in its absence, consume the trailing line + if newline_start_idx is not None and newline_end_idx is not None: + self.idx = newline_end_idx self.line += 1 self.col = 0 - self._prev_line_end_idx = idx_before - self._prev_line_start_idx = self._line_start_idx - self._line_start_idx = self.idx + self.is_newline = True + self.prev_line_start_idx = self.line_start_idx + self.prev_line_end_idx = newline_start_idx + self.line_start_idx = newline_end_idx else: - self.col += 1 + self.idx = self._len + self.col = self._len - self.line_start_idx + self.is_newline = False return True def process_all(self): """ Processes all characters in the text, updating the line and column numbers accordingly. """ - while self.step(): + while self.step_line(): pass def _get_last_line(self, with_end: bool) -> str: """ Returns the last line processed, optionally including the newline character(s) at the end """ - start_idx = self._prev_line_start_idx - end_idx = self._prev_line_end_idx if not with_end else self._line_start_idx + start_idx = self.prev_line_start_idx + end_idx = self.prev_line_end_idx if not with_end else self.line_start_idx return self._chars[start_idx:end_idx] def process_all_gather_lines(self, with_ends: bool) -> list[str]: @@ -130,12 +138,14 @@ class TextStepper: :return: the list of lines """ lines = [] - while self.step(): - if self._is_newline: + while self.step_line(): + if self.is_newline: lines.append(self._get_last_line(with_end=with_ends)) + # add the last line (which was not followed by a newline), even if empty - last_line = self._chars[self._line_start_idx :] + last_line = self._chars[self.line_start_idx :] lines.append(last_line) + return lines @@ -151,10 +161,25 @@ class TextUtils: :param index: the 0-based index in the text :return: a tuple (0-based line number, 0-based column number) corresponding to the index in the text """ + # step over full lines; once the line containing the index has been processed, compute + # the position from the difference to the respective line's start index text_stepper = TextStepper(text) - while text_stepper.idx < index: - if not text_stepper.step(): - raise InvalidTextLocationError + while text_stepper.step_line(): + if text_stepper.idx > index: + if text_stepper.is_newline: + # position was stepped over as part of the previous line + if index > text_stepper.prev_line_end_idx: + # edge case: the index points into a multi-character newline sequence ("\r\n"); + # map this to the beginning of the following line + return text_stepper.line, 0 + return text_stepper.line - 1, index - text_stepper.prev_line_start_idx + else: + # position was stepped over as part of the current line (which was not followed by a newline) + return text_stepper.line, index - text_stepper.line_start_idx + + # handle the case where the end of the text was reached without stepping past the index + if index > text_stepper.idx: + raise InvalidTextLocationError return text_stepper.line, text_stepper.col @classmethod @@ -174,12 +199,13 @@ class TextUtils: :param col: the 0-based column number :return: the corresponding 0-based index in the text """ + # step over full lines until the requested line is reached text_stepper = TextStepper(text) while text_stepper.line < line: - if not text_stepper.step(): + if not text_stepper.step_line(): raise InvalidTextLocationError - idx = text_stepper.idx + col - return idx + + return text_stepper.line_start_idx + col @staticmethod def _get_updated_position_from_line_and_column_and_edit(l: int, c: int, text_to_be_inserted: str) -> tuple[int, int]: diff --git a/test/solidlsp/test_text_utils.py b/test/solidlsp/test_text_utils.py index fa937242..4204f7e9 100644 --- a/test/solidlsp/test_text_utils.py +++ b/test/solidlsp/test_text_utils.py @@ -20,6 +20,8 @@ class TestTextUtils: assert lines[i][len(self.LINE) :] == ending def test_line_col_from_idx(self): + assert TextUtils.get_line_col_from_index(self.LINE, 0) == (0, 0) + assert TextUtils.get_line_col_from_index(self.LINE, 1) == (0, 1) assert TextUtils.get_line_col_from_index(self.TEXT, 0) == (0, 0) assert TextUtils.get_line_col_from_index(self.TEXT, 1) == (0, 1) assert TextUtils.get_line_col_from_index(self.TEXT, 3 + 1 + 1) == (1, 1) From c80d76abe9f3619ad18e7e83e937f03da0cb3a4b Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Wed, 15 Jul 2026 18:45:18 +0200 Subject: [PATCH 4/5] Fix edge case in insert_text_at_position when inserting in line beyond actual content (insertion coordinates were not adjusted to true end coordinates) --- src/solidlsp/ls_utils.py | 5 ++++- test/solidlsp/test_text_utils.py | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/solidlsp/ls_utils.py b/src/solidlsp/ls_utils.py index 0f8ef140..774d3741 100644 --- a/src/solidlsp/ls_utils.py +++ b/src/solidlsp/ls_utils.py @@ -263,9 +263,12 @@ class TextUtils: num_lines_in_text = text_stepper.line + 1 max_line = num_lines_in_text - 1 if line == max_line + 1 and col == 0: # trying to insert at new line after full text - # insert at end, adding missing newline + # insert at end, adding missing newline and adjusting insertion position + # to the actual end coordinates of the text change_index = len(text) text_to_be_inserted = "\n" + text_to_be_inserted + line = text_stepper.line + col = text_stepper.col else: raise new_text = text[:change_index] + text_to_be_inserted + text[change_index:] diff --git a/test/solidlsp/test_text_utils.py b/test/solidlsp/test_text_utils.py index 4204f7e9..062125c4 100644 --- a/test/solidlsp/test_text_utils.py +++ b/test/solidlsp/test_text_utils.py @@ -32,3 +32,19 @@ class TestTextUtils: assert TextUtils.get_index_from_line_col(self.TEXT, 0, 1) == 1 assert TextUtils.get_index_from_line_col(self.TEXT, 1, 1) == 3 + 1 + 1 assert TextUtils.get_index_from_line_col(self.TEXT, 2, 1) == 3 + 1 + 3 + 2 + 1 + + def test_insert_text_at_index(self): + insertion = "XXX" + new_text, l, c = TextUtils.insert_text_at_position(self.TEXT, 0, 1, insertion) + assert (l, c) == (0, 1 + len(insertion)) + assert new_text.startswith("0XXX12") + + def test_insert_text_in_next_line_beyond_content(self): + """ + Test inserting text at a line index 1 beyond the actual number of lines. + This case is specifically handled as an edge case in the implementation. + """ + insertion = "XXX" + new_text, l, c = TextUtils.insert_text_at_position(self.TEXT, 4, 0, insertion) + assert (l, c) == (4, len(insertion)) + assert new_text == self.TEXT + "\n" + insertion From ba5ade0d4e7929c54f0c42361302b4f28d329e20 Mon Sep 17 00:00:00 2001 From: Dominik Jain Date: Wed, 15 Jul 2026 18:50:07 +0200 Subject: [PATCH 5/5] Add location information when raising InvalidTextLocationError --- src/solidlsp/ls_utils.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/solidlsp/ls_utils.py b/src/solidlsp/ls_utils.py index 774d3741..eafc3b37 100644 --- a/src/solidlsp/ls_utils.py +++ b/src/solidlsp/ls_utils.py @@ -179,7 +179,7 @@ class TextUtils: # handle the case where the end of the text was reached without stepping past the index if index > text_stepper.idx: - raise InvalidTextLocationError + raise InvalidTextLocationError(f"{index=}") return text_stepper.line, text_stepper.col @classmethod @@ -203,7 +203,7 @@ class TextUtils: text_stepper = TextStepper(text) while text_stepper.line < line: if not text_stepper.step_line(): - raise InvalidTextLocationError + raise InvalidTextLocationError("{line=}, {col=}") return text_stepper.line_start_idx + col