From a8c3b8700bc05222b8e6ca855c618f30a8c8eac8 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sat, 28 Sep 2024 20:19:10 +0100 Subject: [PATCH 1/6] fix split cells --- CHANGELOG.md | 5 +++++ rich/segment.py | 10 ++++++++-- tests/test_segment.py | 8 ++++++++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 59e8a1879..602f38093 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Rich will display tracebacks with finely grained error locations on python 3.11+ https://github.com/Textualize/rich/pull/3486 + +### Fixed + +- Fixed issue with Segment._split_cells + ## [13.8.1] - 2024-09-10 ### Fixed diff --git a/rich/segment.py b/rich/segment.py index 603d5097f..413a9c4c4 100644 --- a/rich/segment.py +++ b/rich/segment.py @@ -109,16 +109,22 @@ def is_control(self) -> bool: @classmethod @lru_cache(1024 * 16) def _split_cells(cls, segment: "Segment", cut: int) -> Tuple["Segment", "Segment"]: + """Split a segment in to two at a given cell position. + + Returns: + A tuple of two segments. + """ text, style, control = segment _Segment = Segment - cell_length = segment.cell_length if cut >= cell_length: return segment, _Segment("", style, control) cell_size = get_character_cell_size - pos = int((cut / cell_length) * (len(text) - 1)) + pos = int((cut / cell_length) * (len(text) - 1)) - 1 + if pos < 0: + pos = 0 before = text[:pos] cell_pos = cell_len(before) diff --git a/tests/test_segment.py b/tests/test_segment.py index 3db60b908..a775ac2f7 100644 --- a/tests/test_segment.py +++ b/tests/test_segment.py @@ -2,6 +2,7 @@ import pytest +from rich.cells import cell_len from rich.segment import ControlType, Segment, SegmentLines, Segments from rich.style import Style @@ -284,6 +285,13 @@ def test_split_cells_emoji(text, split, result): assert Segment(text).split_cells(split) == result +def test_split_cells_mixed() -> None: + test = Segment("早乙女リリエル (CV: 徳井青）") + for position in range(1, test.cell_length): + left, right = Segment.split_cells(test, position) + assert cell_len(left.text) == position + + def test_segment_lines_renderable(): lines = [[Segment("hello"), Segment(" "), Segment("world")], [Segment("foo")]] segment_lines = SegmentLines(lines) From 13147d69c77de998648bae6849ff399bcac64e96 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sat, 28 Sep 2024 20:20:58 +0100 Subject: [PATCH 2/6] changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 602f38093..c40bc9483 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- Fixed issue with Segment._split_cells +- Fixed issue with Segment._split_cells https://github.com/Textualize/rich/pull/3506 ## [13.8.1] - 2024-09-10 From 78701311feb074c77b3067c93e830be210c1723f Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sat, 28 Sep 2024 20:23:25 +0100 Subject: [PATCH 3/6] comment --- tests/test_segment.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_segment.py b/tests/test_segment.py index a775ac2f7..e4c688d6a 100644 --- a/tests/test_segment.py +++ b/tests/test_segment.py @@ -286,6 +286,8 @@ def test_split_cells_emoji(text, split, result): def test_split_cells_mixed() -> None: + """Check that split cells splits on cell positions.""" + # Caused https://github.com/Textualize/textual/issues/4996 in Textual test = Segment("早乙女リリエル (CV: 徳井青）") for position in range(1, test.cell_length): left, right = Segment.split_cells(test, position) From 0633a8b41102f733ad5d97aee2126e3b091ef250 Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sat, 28 Sep 2024 20:25:31 +0100 Subject: [PATCH 4/6] docstring --- rich/segment.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/rich/segment.py b/rich/segment.py index 413a9c4c4..aa2394cae 100644 --- a/rich/segment.py +++ b/rich/segment.py @@ -111,6 +111,13 @@ def is_control(self) -> bool: def _split_cells(cls, segment: "Segment", cut: int) -> Tuple["Segment", "Segment"]: """Split a segment in to two at a given cell position. + Note that splitting a double-width character, may result in that character turning + into two spaces. + + Args: + segment (Segment): A segment to split. + cut (int): A cell position to cut on. + Returns: A tuple of two segments. """ From 3a6c86efc3da3ef4abaf1ad41b5f10f79b6abaeb Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sat, 28 Sep 2024 20:30:53 +0100 Subject: [PATCH 5/6] simplify --- rich/segment.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rich/segment.py b/rich/segment.py index aa2394cae..86c8c9a9c 100644 --- a/rich/segment.py +++ b/rich/segment.py @@ -129,7 +129,7 @@ def _split_cells(cls, segment: "Segment", cut: int) -> Tuple["Segment", "Segment cell_size = get_character_cell_size - pos = int((cut / cell_length) * (len(text) - 1)) - 1 + pos = int((cut / cell_length) * (len(text))) - 1 if pos < 0: pos = 0 From 23c64f16afc83d267e79fbb9df964dda34c1529c Mon Sep 17 00:00:00 2001 From: Will McGugan Date: Sat, 28 Sep 2024 20:34:29 +0100 Subject: [PATCH 6/6] more tests --- tests/test_segment.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/test_segment.py b/tests/test_segment.py index e4c688d6a..4dac53c8c 100644 --- a/tests/test_segment.py +++ b/tests/test_segment.py @@ -292,6 +292,25 @@ def test_split_cells_mixed() -> None: for position in range(1, test.cell_length): left, right = Segment.split_cells(test, position) assert cell_len(left.text) == position + assert cell_len(right.text) == test.cell_length - position + + +def test_split_cells_doubles() -> None: + """Check that split cells splits on cell positions with all double width characters.""" + test = Segment("早" * 20) + for position in range(1, test.cell_length): + left, right = Segment.split_cells(test, position) + assert cell_len(left.text) == position + assert cell_len(right.text) == test.cell_length - position + + +def test_split_cells_single() -> None: + """Check that split cells splits on cell positions with all single width characters.""" + test = Segment("A" * 20) + for position in range(1, test.cell_length): + left, right = Segment.split_cells(test, position) + assert cell_len(left.text) == position + assert cell_len(right.text) == test.cell_length - position def test_segment_lines_renderable():