Fixing click targeting/tab expansion interaction

CHANGELOG.md

@@ -6,6 +6,12 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).
 
 
+## Unreleased
+
+### Fixed
+
+- Fixed mouse targeting issue in `TextArea` when tabs were not fully expanded https://github.com/Textualize/textual/pull/3725
+
 ## [0.42.0] - 2023-11-22
 
 ### Fixed

src/textual/_cells.py

@@ -1,10 +1,44 @@
 from typing import Callable
 
-__all__ = ["cell_len"]
+from textual.expand_tabs import get_tab_widths
+
+__all__ = ["cell_len", "cell_width_to_column_index"]
 
 
 cell_len: Callable[[str], int]
 try:
     from rich.cells import cached_cell_len as cell_len
 except ImportError:
     from rich.cells import cell_len
+
+
+def cell_width_to_column_index(line: str, cell_width: int, tab_width: int) -> int:
+    """Retrieve the column index corresponding to the given cell width.
+
+    Args:
+        line: The line of text to search within.
+        cell_width: The cell width to convert to column index.
+        tab_width: The tab stop width to expand tabs contained within the line.
+
+    Returns:
+        The column corresponding to the cell width.
+    """
+    column_index = 0
+    total_cell_offset = 0
+    for part, expanded_tab_width in get_tab_widths(line, tab_width):
+        # Check if the click landed on a character within this part.
+        for character in part:
+            total_cell_offset += cell_len(character)
+            if total_cell_offset > cell_width:
+                return column_index
+            column_index += 1
+
+        # Account for the appearance of the tab character for this part
+        total_cell_offset += expanded_tab_width
+        # Check if the click falls within the boundary of the expanded tab.
+        if total_cell_offset > cell_width:
+            return column_index
+
+        column_index += 1
+
+    return len(line)

src/textual/expand_tabs.py

@@ -7,37 +7,63 @@
 _TABS_SPLITTER_RE = re.compile(r"(.*?\t|.+?$)")
 
 
-def expand_tabs_inline(line: str, tab_size: int = 4) -> str:
-    """Expands tabs, taking into account double cell characters.
+def get_tab_widths(line: str, tab_size: int = 4) -> list[tuple[str, int]]:
+    """Splits a string line into tuples (str, int).
+
+    Each tuple represents a section of the line which precedes a tab character.
+    The string is the string text that appears before the tab character (excluding the tab).
+    The integer is the width that the tab character is expanded to.
 
     Args:
         line: The text to expand tabs in.
         tab_size: Number of cells in a tab.
+
     Returns:
-        New string with tabs replaced with spaces.
+        A list of tuples representing the line split on tab characters,
+            and the widths of the tabs after tab expansion is applied.
+
     """
-    if "\t" not in line:
-        return line
-    new_line_parts: list[str] = []
-    add_part = new_line_parts.append
+
+    parts: list[tuple[str, int]] = []
+    add_part = parts.append
     cell_position = 0
-    parts = _TABS_SPLITTER_RE.findall(line)
+    matches = _TABS_SPLITTER_RE.findall(line)
+
+    for match in matches:
+        expansion_width = 0
+        if match.endswith("\t"):
+            # Remove the tab, and check the width of the rest of the line.
+            match = match[:-1]
+            cell_position += cell_len(match)
 
-    for part in parts:
-        if part.endswith("\t"):
-            part = f"{part[:-1]} "
-            cell_position += cell_len(part)
+            # Now move along the line by the width of the tab.
             tab_remainder = cell_position % tab_size
-            if tab_remainder:
-                spaces = tab_size - tab_remainder
-                part += spaces * " "
-        add_part(part)
+            expansion_width = tab_size - tab_remainder
+            cell_position += expansion_width
+
+        add_part((match, expansion_width))
+
+    return parts
 
-    return "".join(new_line_parts)
+
+def expand_tabs_inline(line: str, tab_size: int = 4) -> str:
+    """Expands tabs, taking into account double cell characters.
+
+    Args:
+        line: The text to expand tabs in.
+        tab_size: Number of cells in a tab.
+    Returns:
+        New string with tabs replaced with spaces.
+    """
+    tab_widths = get_tab_widths(line, tab_size)
+    return "".join(
+        [part + expansion_width * " " for part, expansion_width in tab_widths]
+    )
 
 
 if __name__ == "__main__":
     print(expand_tabs_inline("\tbar"))
+    print(expand_tabs_inline("\tbar\t"))
     print(expand_tabs_inline("1\tbar"))
     print(expand_tabs_inline("12\tbar"))
     print(expand_tabs_inline("123\tbar"))

src/textual/widgets/_text_area.py

@@ -33,7 +33,7 @@
     from tree_sitter import Language
 
 from textual import events, log
-from textual._cells import cell_len
+from textual._cells import cell_len, cell_width_to_column_index
 from textual.binding import Binding
 from textual.events import Message, MouseEvent
 from textual.geometry import Offset, Region, Size, Spacing, clamp
@@ -1118,14 +1118,8 @@ def cell_width_to_column_index(self, cell_width: int, row_index: int) -> int:
         Returns:
             The column corresponding to the cell width on that row.
         """
-        tab_width = self.indent_width
-        total_cell_offset = 0
         line = self.document[row_index]
-        for column_index, character in enumerate(line):
-            total_cell_offset += cell_len(expand_tabs_inline(character, tab_width))
-            if total_cell_offset >= cell_width + 1:
-                return column_index
-        return len(line)
+        return cell_width_to_column_index(line, cell_width, self.indent_width)
 
     def clamp_visitable(self, location: Location) -> Location:
         """Clamp the given location to the nearest visitable location.

tests/test_expand_tabs.py

@@ -0,0 +1,25 @@
+import pytest
+
+from textual.expand_tabs import expand_tabs_inline
+
+
+@pytest.mark.parametrize(
+    "line, expanded_line",
+    [
+        (" b ar ", " b ar "),
+        ("\tbar", "    bar"),
+        ("\tbar\t", "    bar "),
+        ("\tr\t", "    r   "),
+        ("1\tbar", "1   bar"),
+        ("12\tbar", "12  bar"),
+        ("123\tbar", "123 bar"),
+        ("1234\tbar", "1234    bar"),
+        ("💩\tbar", "💩  bar"),
+        ("💩💩\tbar", "💩💩    bar"),
+        ("💩💩💩\tbar", "💩💩💩  bar"),
+        ("F💩\tbar", "F💩 bar"),
+        ("F💩O\tbar", "F💩O    bar"),
+    ],
+)
+def test_expand_tabs_inline(line, expanded_line):
+    assert expand_tabs_inline(line) == expanded_line