diff --git a/cmd2/argparse_completer.py b/cmd2/argparse_completer.py index 0d32a2a32..26e96e27d 100644 --- a/cmd2/argparse_completer.py +++ b/cmd2/argparse_completer.py @@ -33,7 +33,6 @@ from .completion import ( CompletionItem, Completions, - all_display_numeric, ) from .constants import INFINITY from .exceptions import CompletionError @@ -647,12 +646,15 @@ def _build_completion_table(self, arg_state: _ArgumentState, completions: Comple # the 3rd or more argument here. destination = destination[min(len(destination) - 1, arg_state.count)] - # Determine if all display values are numeric so we can right-align them - all_nums = all_display_numeric(completions.items) - # Build header row rich_columns: list[Column] = [] - rich_columns.append(Column(destination.upper(), justify="right" if all_nums else "left", no_wrap=True)) + rich_columns.append( + Column( + destination.upper(), + justify="right" if completions.numeric_display else "left", + no_wrap=True, + ) + ) rich_columns.extend( column if isinstance(column, Column) else Column(column, overflow="fold") for column in table_columns ) diff --git a/cmd2/completion.py b/cmd2/completion.py index 6364be4b4..8065f5a47 100644 --- a/cmd2/completion.py +++ b/cmd2/completion.py @@ -3,7 +3,6 @@ import re import sys from collections.abc import ( - Collection, Iterable, Iterator, Sequence, @@ -31,26 +30,15 @@ from . import rich_utils as ru -# Regular expression to identify strings which we should sort numerically -NUMERIC_RE = re.compile( - r""" - ^ # Start of string - [-+]? # Optional sign - (?: # Start of non-capturing group - \d+\.?\d* # Matches 123 or 123. or 123.45 - | # OR - \.\d+ # Matches .45 - ) # End of group - $ # End of string -""", - re.VERBOSE, -) - @dataclass(frozen=True, slots=True, kw_only=True) class CompletionItem: """A single completion result.""" + # Regular expression to identify whitespace characters that are rendered as + # control sequences (like ^J or ^I) in the completion menu. + _CONTROL_WHITESPACE_RE = re.compile(r'\r\n|[\n\r\t\f\v]') + # The underlying object this completion represents (e.g., str, int, Path). # This is used to support argparse choices validation. value: Any = field(kw_only=False) @@ -76,6 +64,18 @@ class CompletionItem: display_plain: str = field(init=False) display_meta_plain: str = field(init=False) + @classmethod + def _clean_display(cls, val: str) -> str: + """Clean a string for display in the completion menu. + + This replaces whitespace characters that are rendered as + control sequences (like ^J or ^I) with spaces. + + :param val: string to be cleaned + :return: the cleaned string + """ + return cls._CONTROL_WHITESPACE_RE.sub(' ', val) + def __post_init__(self) -> None: """Finalize the object after initialization.""" # Derive text from value if it wasn't explicitly provided @@ -86,7 +86,11 @@ def __post_init__(self) -> None: if not self.display: object.__setattr__(self, "display", self.text) - # Pre-calculate plain text versions by stripping ANSI sequences. + # Clean display and display_meta + object.__setattr__(self, "display", self._clean_display(self.display)) + object.__setattr__(self, "display_meta", self._clean_display(self.display_meta)) + + # Create plain text versions by stripping ANSI sequences. # These are stored as attributes for fast access during sorting/filtering. object.__setattr__(self, "display_plain", su.strip_style(self.display)) object.__setattr__(self, "display_meta_plain", su.strip_style(self.display_meta)) @@ -135,6 +139,21 @@ def __hash__(self) -> int: class CompletionResultsBase: """Base class for results containing a collection of CompletionItems.""" + # Regular expression to identify strings that we should sort numerically + _NUMERIC_RE = re.compile( + r""" + ^ # Start of string + [-+]? # Optional sign + (?: # Start of non-capturing group + \d+\.?\d* # Matches 123 or 123. or 123.45 + | # OR + \.\d+ # Matches .45 + ) # End of group + $ # End of string + """, + re.VERBOSE, + ) + # The collection of CompletionItems. This is stored internally as a tuple. items: Sequence[CompletionItem] = field(default_factory=tuple, kw_only=False) @@ -142,13 +161,22 @@ class CompletionResultsBase: # If False, items will be sorted by their display value during initialization. is_sorted: bool = False + # True if every item in this collection has a numeric display string. + # Used for sorting and alignment. + numeric_display: bool = field(init=False) + def __post_init__(self) -> None: """Finalize the object after initialization.""" from . import utils unique_items = utils.remove_duplicates(self.items) + + # Determine if all items have numeric display strings + numeric_display = bool(unique_items) and all(self._NUMERIC_RE.match(i.display_plain) for i in unique_items) + object.__setattr__(self, "numeric_display", numeric_display) + if not self.is_sorted: - if all_display_numeric(unique_items): + if self.numeric_display: # Sort numerically unique_items.sort(key=lambda item: float(item.display_plain)) else: @@ -249,8 +277,3 @@ class Completions(CompletionResultsBase): # The quote character to use if adding an opening or closing quote to the matches. _quote_char: str = "" - - -def all_display_numeric(items: Collection[CompletionItem]) -> bool: - """Return True if items is non-empty and every item.display_plain value is a numeric string.""" - return bool(items) and all(NUMERIC_RE.match(item.display_plain) for item in items) diff --git a/tests/test_completion.py b/tests/test_completion.py index 1492844a3..02018ba3c 100644 --- a/tests/test_completion.py +++ b/tests/test_completion.py @@ -19,7 +19,6 @@ Completions, utils, ) -from cmd2.completion import all_display_numeric from .conftest import ( normalize, @@ -877,7 +876,7 @@ def test_is_sorted() -> None: @pytest.mark.parametrize( - ('values', 'all_nums'), + ('values', 'numeric_display'), [ ([2, 3], True), ([2, 3.7], True), @@ -889,11 +888,10 @@ def test_is_sorted() -> None: (["\x1b[31mNOT_STRING\x1b[0m", "\x1b[32m9.2\x1b[0m"], False), ], ) -def test_all_display_numeric(values: list[int | float | str], all_nums: bool) -> None: - """Test that all_display_numeric() evaluates the display_plain field.""" - - items = [CompletionItem(v) for v in values] - assert all_display_numeric(items) == all_nums +def test_numeric_display(values: list[int | float | str], numeric_display: bool) -> None: + """Test setting of the Completions.numeric_display field.""" + completions = Completions.from_values(values) + assert completions.numeric_display == numeric_display def test_remove_duplicates() -> None: @@ -932,6 +930,25 @@ def test_plain_fields() -> None: assert completion_item.display_meta_plain == "A tasty apple" +def test_clean_display() -> None: + """Test display string cleaning in CompletionItem.""" + # Test all problematic characters being replaced by a single space. + # Also verify that \r\n is replaced by a single space. + display = "str1\r\nstr2\nstr3\rstr4\tstr5\fstr6\vstr7" + expected = "str1 str2 str3 str4 str5 str6 str7" + + # Since display defaults to text if not provided, we test both text and display fields + completion_item = CompletionItem("item", display=display, display_meta=display) + assert completion_item.display == expected + assert completion_item.display_meta == expected + + # Verify that text derived display is also sanitized + text = "item\nwith\nnewlines" + expected_text_display = "item with newlines" + completion_item = CompletionItem(text) + assert completion_item.display == expected_text_display + + def test_styled_completion_sort() -> None: """Test that sorting is done with the display_plain field."""