From fc09fb46b9fed460adefcb149040fff47b11b12e Mon Sep 17 00:00:00 2001 From: Jakob Zahn Date: Wed, 4 Jun 2025 08:34:59 +0200 Subject: [PATCH 1/4] Remove `maintain_selection_offset` flag --- src/textual/document/_edit.py | 14 ++++--------- src/textual/widgets/_text_area.py | 30 ++++++---------------------- tests/text_area/test_edit_via_api.py | 7 +++---- 3 files changed, 13 insertions(+), 38 deletions(-) diff --git a/src/textual/document/_edit.py b/src/textual/document/_edit.py index dc9c04aeba..5ebd1dfedd 100644 --- a/src/textual/document/_edit.py +++ b/src/textual/document/_edit.py @@ -22,9 +22,6 @@ class Edit: to_location: Location """The end location of the insert""" - maintain_selection_offset: bool - """If True, the selection will maintain its offset to the replacement range.""" - _original_selection: Selection | None = field(init=False, default=None) """The Selection when the edit was originally performed, to be restored on undo.""" @@ -92,13 +89,10 @@ def do(self, text_area: TextArea, record_selection: bool = True) -> EditResult: else selection_end_row ) - if self.maintain_selection_offset: - self._updated_selection = Selection( - start=(target_selection_start_row, target_selection_start_column), - end=(target_selection_end_row, target_selection_end_column), - ) - else: - self._updated_selection = Selection.cursor(edit_result.end_location) + self._updated_selection = Selection( + start=(target_selection_start_row, target_selection_start_column), + end=(target_selection_end_row, target_selection_end_column), + ) self._edit_result = edit_result return edit_result diff --git a/src/textual/widgets/_text_area.py b/src/textual/widgets/_text_area.py index f180c28f30..c1a41d60fe 100644 --- a/src/textual/widgets/_text_area.py +++ b/src/textual/widgets/_text_area.py @@ -2110,55 +2110,41 @@ def insert( self, text: str, location: Location | None = None, - *, - maintain_selection_offset: bool = True, ) -> EditResult: """Insert text into the document. Args: text: The text to insert. location: The location to insert text, or None to use the cursor location. - maintain_selection_offset: If True, the active Selection will be updated - such that the same text is selected before and after the selection, - if possible. Otherwise, the cursor will jump to the end point of the - edit. Returns: An `EditResult` containing information about the edit. """ if location is None: location = self.cursor_location - return self.edit(Edit(text, location, location, maintain_selection_offset)) + return self.edit(Edit(text, location, location)) def delete( self, start: Location, end: Location, - *, - maintain_selection_offset: bool = True, ) -> EditResult: """Delete the text between two locations in the document. Args: start: The start location. end: The end location. - maintain_selection_offset: If True, the active Selection will be updated - such that the same text is selected before and after the selection, - if possible. Otherwise, the cursor will jump to the end point of the - edit. Returns: An `EditResult` containing information about the edit. """ - return self.edit(Edit("", start, end, maintain_selection_offset)) + return self.edit(Edit("", start, end)) def replace( self, insert: str, start: Location, end: Location, - *, - maintain_selection_offset: bool = True, ) -> EditResult: """Replace text in the document with new text. @@ -2166,15 +2152,11 @@ def replace( insert: The text to insert. start: The start location end: The end location. - maintain_selection_offset: If True, the active Selection will be updated - such that the same text is selected before and after the selection, - if possible. Otherwise, the cursor will jump to the end point of the - edit. Returns: An `EditResult` containing information about the edit. """ - return self.edit(Edit(insert, start, end, maintain_selection_offset)) + return self.edit(Edit(insert, start, end)) def clear(self) -> EditResult: """Delete all text from the document. @@ -2182,7 +2164,7 @@ def clear(self) -> EditResult: Returns: An EditResult relating to the deletion of all content. """ - return self.delete((0, 0), self.document.end, maintain_selection_offset=False) + return self.delete((0, 0), self.document.end) def _delete_via_keyboard( self, @@ -2200,7 +2182,7 @@ def _delete_via_keyboard( """ if self.read_only: return None - return self.delete(start, end, maintain_selection_offset=False) + return self.delete(start, end) def _replace_via_keyboard( self, @@ -2220,7 +2202,7 @@ def _replace_via_keyboard( """ if self.read_only: return None - return self.replace(insert, start, end, maintain_selection_offset=False) + return self.replace(insert, start, end) def action_delete_left(self) -> None: """Deletes the character to the left of the cursor and updates the cursor location. diff --git a/tests/text_area/test_edit_via_api.py b/tests/text_area/test_edit_via_api.py index e732680f0f..47bed276d3 100644 --- a/tests/text_area/test_edit_via_api.py +++ b/tests/text_area/test_edit_via_api.py @@ -56,7 +56,7 @@ async def test_insert_text_start(): async with app.run_test(): text_area = app.query_one(TextArea) text_area.move_cursor((0, 5)) - text_area.insert("Hello", location=(0, 0), maintain_selection_offset=False) + text_area.insert("Hello", location=(0, 0)) assert text_area.text == "Hello" + TEXT assert text_area.selection == Selection.cursor((0, 5)) @@ -181,7 +181,6 @@ async def test_insert_text_non_cursor_location_dont_maintain_offset(): result = text_area.insert( "Hello", location=(4, 0), - maintain_selection_offset=False, ) assert result == EditResult( @@ -200,7 +199,7 @@ async def test_insert_multiline_text(): async with app.run_test(): text_area = app.query_one(TextArea) text_area.move_cursor((2, 5)) - text_area.insert("Hello,\nworld!", maintain_selection_offset=False) + text_area.insert("Hello,\nworld!") expected_content = """\ I must not fear. Fear is the mind-killer. @@ -331,7 +330,7 @@ async def test_delete_within_line_dont_maintain_offset(): app = TextAreaApp() async with app.run_test(): text_area = app.query_one(TextArea) - text_area.delete((0, 6), (0, 10), maintain_selection_offset=False) + text_area.delete((0, 6), (0, 10)) expected_text = """\ I must fear. Fear is the mind-killer. From 2f3327630a1d421c07c83c131237f716d3d8ba9f Mon Sep 17 00:00:00 2001 From: Jakob Zahn Date: Tue, 10 Jun 2025 13:48:24 +0200 Subject: [PATCH 2/4] Rework selection updates --- src/textual/document/_document.py | 18 +++++ src/textual/document/_edit.py | 112 +++++++++++++++++++----------- 2 files changed, 88 insertions(+), 42 deletions(-) diff --git a/src/textual/document/_document.py b/src/textual/document/_document.py index 47e87eb09b..de3088c377 100644 --- a/src/textual/document/_document.py +++ b/src/textual/document/_document.py @@ -466,3 +466,21 @@ def is_empty(self) -> bool: """Return True if the selection has 0 width, i.e. it's just a cursor.""" start, end = self return start == end + + def __contains__(self, location: Location) -> bool: + start, end = self + start, end = sorted((start, end)) + return start <= location <= end + + def __gt__(self, location: Location) -> bool: + return location < max(self.start, self.end) + + def __ge__(self, location: Location) -> bool: + return location <= max(self.start, self.end) + + def __lt__(self, location: Location) -> bool: + return location > min(self.start, self.end) + + def __le__(self, location: Location) -> bool: + return location >= min(self.start, self.end) + diff --git a/src/textual/document/_edit.py b/src/textual/document/_edit.py index 5ebd1dfedd..99f4bcc2d1 100644 --- a/src/textual/document/_edit.py +++ b/src/textual/document/_edit.py @@ -31,6 +31,43 @@ class Edit: _edit_result: EditResult | None = field(init=False, default=None) """The result of doing the edit.""" + def _update_location(self, location: Location, delete: Selection, insert: Selection, target: Location) -> Location: + """Move a given location with respect to deletion and insertion ranges of an edit. + + Args: + location: Location before the edit. + delete: Range which is deleted during the edit. + insert: Range which is inserted during the edit. + target: Returned location when `location` is within the deletion range, + typically the start or the end of the insertion range. + + Returns: + Location after the edit. + """ + loc_ = location + del_ = delete + ins_ = insert + + if loc_ < del_: + # `loc_` is not affected by the edit and thus does not change + pass + elif loc_ in del_: + # `loc_` is within the deletion range and set to the `target` + loc_ = target + elif loc_ > del_: + # `loc_` is shifted by the difference in length + # between delete and insert operations + shift = (ins_.end[0] - del_.end[0], ins_.end[1] - del_.end[1]) + + # only shift columns when edit happened in the same row `loc_` is also in + if del_.end[0] < loc_[0]: + shift = (shift[0], 0) + + loc_ = (loc_[0] + shift[0], loc_[1] + shift[1]) + + return loc_ + + def do(self, text_area: TextArea, record_selection: bool = True) -> EditResult: """Perform the edit operation. @@ -45,7 +82,7 @@ def do(self, text_area: TextArea, record_selection: bool = True) -> EditResult: if record_selection: self._original_selection = text_area.selection - text = self.text + edit_result = text_area.document.replace_range(self.top, self.bottom, self.text) # This code is mostly handling how we adjust TextArea.selection # when an edit is made to the document programmatically. @@ -53,48 +90,39 @@ def do(self, text_area: TextArea, record_selection: bool = True) -> EditResult: # position in the document even if an insert happens before # their cursor position. - edit_bottom_row, edit_bottom_column = self.bottom - - selection_start, selection_end = text_area.selection - selection_start_row, selection_start_column = selection_start - selection_end_row, selection_end_column = selection_end - - edit_result = text_area.document.replace_range(self.top, self.bottom, text) - - new_edit_to_row, new_edit_to_column = edit_result.end_location - - column_offset = new_edit_to_column - edit_bottom_column - target_selection_start_column = ( - selection_start_column + column_offset - if edit_bottom_row == selection_start_row - and edit_bottom_column <= selection_start_column - else selection_start_column - ) - target_selection_end_column = ( - selection_end_column + column_offset - if edit_bottom_row == selection_end_row - and edit_bottom_column <= selection_end_column - else selection_end_column - ) - - row_offset = new_edit_to_row - edit_bottom_row - target_selection_start_row = ( - selection_start_row + row_offset - if edit_bottom_row <= selection_start_row - else selection_start_row - ) - target_selection_end_row = ( - selection_end_row + row_offset - if edit_bottom_row <= selection_end_row - else selection_end_row - ) - - self._updated_selection = Selection( - start=(target_selection_start_row, target_selection_start_column), - end=(target_selection_end_row, target_selection_end_column), - ) - + # use the original selection for proper history undo/redo operations + start, end = self._original_selection + + delete = Selection(self.top, self.bottom) + insert = Selection(self.top, edit_result.end_location) + + if (start in delete) and (end in delete): + # the current selection has been deleted, i.e. is within the deletion range; + # reset cursor to end of edit, i.e. insert range + start, end = insert.end, insert.end + else: + # reverse target locations if the current selection is reversed + if start > end: + target_start, target_end = insert.start, insert.end + else: + target_start, target_end = insert.end, insert.start + + ## start + # before edit - no-op + # within edit - shift to end of edit + # after edit - shift by edit length + + ## end + # before edit - no-op + # within edit - shift to start of edit + # after edit - shift by edit length + + start = self._update_location(start, delete, insert, target_start) + end = self._update_location(end, delete, insert, target_end) + + self._updated_selection = Selection(start, end) self._edit_result = edit_result + return edit_result def undo(self, text_area: TextArea) -> EditResult: From c9d194b9173b8aab1c7efc43ad44261f6c1b9941 Mon Sep 17 00:00:00 2001 From: Jakob Zahn Date: Tue, 10 Jun 2025 13:54:18 +0200 Subject: [PATCH 3/4] Adapt tests for removed `maintain_selection_offset` flag and reworked selection update logic --- tests/text_area/test_edit_via_api.py | 30 +---------------------- tests/text_area/test_edit_via_bindings.py | 21 ++++++++++++++-- 2 files changed, 20 insertions(+), 31 deletions(-) diff --git a/tests/text_area/test_edit_via_api.py b/tests/text_area/test_edit_via_api.py index 47bed276d3..c8709a3266 100644 --- a/tests/text_area/test_edit_via_api.py +++ b/tests/text_area/test_edit_via_api.py @@ -48,19 +48,6 @@ async def test_insert_text_start_maintain_selection_offset(): assert text_area.selection == Selection.cursor((0, 10)) -async def test_insert_text_start(): - """The document is correctly updated on inserting at the start. - If we don't maintain the selection offset, the cursor jumps - to the end of the edit and the selection is empty.""" - app = TextAreaApp() - async with app.run_test(): - text_area = app.query_one(TextArea) - text_area.move_cursor((0, 5)) - text_area.insert("Hello", location=(0, 0)) - assert text_area.text == "Hello" + TEXT - assert text_area.selection == Selection.cursor((0, 5)) - - async def test_insert_empty_string(): app = TextAreaApp() async with app.run_test(): @@ -191,7 +178,7 @@ async def test_insert_text_non_cursor_location_dont_maintain_offset(): # Since maintain_selection_offset is False, the selection # is reset to a cursor and goes to the end of the insert. - assert text_area.selection == Selection.cursor((4, 5)) + assert text_area.selection == Selection((2, 3), (3, 5)) async def test_insert_multiline_text(): @@ -326,21 +313,6 @@ async def test_delete_within_line(): assert text_area.text == expected_text -async def test_delete_within_line_dont_maintain_offset(): - app = TextAreaApp() - async with app.run_test(): - text_area = app.query_one(TextArea) - text_area.delete((0, 6), (0, 10)) - expected_text = """\ -I must fear. -Fear is the mind-killer. -Fear is the little-death that brings total obliteration. -I will face my fear. -""" - assert text_area.selection == Selection.cursor((0, 6)) # cursor moved - assert text_area.text == expected_text - - async def test_delete_multiple_lines_selection_above(): app = TextAreaApp() async with app.run_test(): diff --git a/tests/text_area/test_edit_via_bindings.py b/tests/text_area/test_edit_via_bindings.py index f6b3f013f4..bfe946669d 100644 --- a/tests/text_area/test_edit_via_bindings.py +++ b/tests/text_area/test_edit_via_bindings.py @@ -301,9 +301,15 @@ async def test_delete_to_end_of_line(selection, expected_result): await pilot.press("ctrl+k") - assert text_area.selection == Selection.cursor(selection.end) assert text_area.text == expected_result + if selection.start < selection.end: + # the selection is not behind the current cursor location + assert text_area.selection == selection + else: + # the selection is behind the current cursor location and thus deleted + assert text_area.selection == Selection.cursor(selection.end) + @pytest.mark.parametrize( "selection,expected_result", @@ -329,8 +335,19 @@ async def test_delete_to_start_of_line(selection, expected_result): await pilot.press("ctrl+u") - assert text_area.selection == Selection.cursor((0, 0)) assert text_area.text == expected_result + if selection.start > selection.end: + # the selection is behind the current cursor location and + # thus shifted by the length of the edit + shift = (0, - selection.end[1]) + start = selection.start[0] + shift[0], selection.start[1] + shift[1] + end = selection.end[0] + shift[0], selection.end[1] + shift[1] + + assert text_area.selection == Selection(start, end) + else: + # the selection is before the current cursor location and + # hence deleted + assert text_area.selection == Selection.cursor((0, 0)) @pytest.mark.parametrize( From 975381d905155dc6506c4b8598bd7077bf939991 Mon Sep 17 00:00:00 2001 From: Jakob Zahn Date: Tue, 10 Jun 2025 14:32:37 +0200 Subject: [PATCH 4/4] Apply `black` formatting --- src/textual/document/_document.py | 1 - src/textual/document/_edit.py | 5 +++-- tests/text_area/test_edit_via_api.py | 4 +--- tests/text_area/test_edit_via_bindings.py | 2 +- 4 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/textual/document/_document.py b/src/textual/document/_document.py index de3088c377..817bfe69ba 100644 --- a/src/textual/document/_document.py +++ b/src/textual/document/_document.py @@ -483,4 +483,3 @@ def __lt__(self, location: Location) -> bool: def __le__(self, location: Location) -> bool: return location >= min(self.start, self.end) - diff --git a/src/textual/document/_edit.py b/src/textual/document/_edit.py index 99f4bcc2d1..699c01d71d 100644 --- a/src/textual/document/_edit.py +++ b/src/textual/document/_edit.py @@ -31,7 +31,9 @@ class Edit: _edit_result: EditResult | None = field(init=False, default=None) """The result of doing the edit.""" - def _update_location(self, location: Location, delete: Selection, insert: Selection, target: Location) -> Location: + def _update_location( + self, location: Location, delete: Selection, insert: Selection, target: Location + ) -> Location: """Move a given location with respect to deletion and insertion ranges of an edit. Args: @@ -67,7 +69,6 @@ def _update_location(self, location: Location, delete: Selection, insert: Select return loc_ - def do(self, text_area: TextArea, record_selection: bool = True) -> EditResult: """Perform the edit operation. diff --git a/tests/text_area/test_edit_via_api.py b/tests/text_area/test_edit_via_api.py index c8709a3266..d9a59f9666 100644 --- a/tests/text_area/test_edit_via_api.py +++ b/tests/text_area/test_edit_via_api.py @@ -102,9 +102,7 @@ async def test_insert_character_near_cursor_maintain_selection_offset( ], ) async def test_insert_newline_around_cursor_maintain_selection_offset( - cursor_location, - insert_location, - cursor_destination + cursor_location, insert_location, cursor_destination ): app = TextAreaApp() async with app.run_test(): diff --git a/tests/text_area/test_edit_via_bindings.py b/tests/text_area/test_edit_via_bindings.py index bfe946669d..9acf4355f3 100644 --- a/tests/text_area/test_edit_via_bindings.py +++ b/tests/text_area/test_edit_via_bindings.py @@ -339,7 +339,7 @@ async def test_delete_to_start_of_line(selection, expected_result): if selection.start > selection.end: # the selection is behind the current cursor location and # thus shifted by the length of the edit - shift = (0, - selection.end[1]) + shift = (0, -selection.end[1]) start = selection.start[0] + shift[0], selection.start[1] + shift[1] end = selection.end[0] + shift[0], selection.end[1] + shift[1]