From a05d0716e3a2844638909ae242323fc167beec0c Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Wed, 1 Apr 2026 13:06:22 -0400 Subject: [PATCH 1/5] Sanitize whitespace characters in CompletionItem display fields to ensure correct rendering in the completion menu. --- cmd2/completion.py | 15 ++++++++++++++- tests/test_completion.py | 19 +++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/cmd2/completion.py b/cmd2/completion.py index 6364be4b4..d368d982f 100644 --- a/cmd2/completion.py +++ b/cmd2/completion.py @@ -76,6 +76,15 @@ class CompletionItem: display_plain: str = field(init=False) display_meta_plain: str = field(init=False) + @staticmethod + def _sanitize_display_string(val: str) -> str: + """Sanitize a string for display in the completion menu. + + This replaces whitespace characters that are rendered as + control sequences (like ^J or ^I) with spaces. + """ + return re.sub(r'\r\n|[\n\r\t\f\v]', ' ', val) + def __post_init__(self) -> None: """Finalize the object after initialization.""" # Derive text from value if it wasn't explicitly provided @@ -86,7 +95,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. + # Sanitize display and display_meta + object.__setattr__(self, "display", self._sanitize_display_string(self.display)) + object.__setattr__(self, "display_meta", self._sanitize_display_string(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)) diff --git a/tests/test_completion.py b/tests/test_completion.py index 1492844a3..99b925190 100644 --- a/tests/test_completion.py +++ b/tests/test_completion.py @@ -932,6 +932,25 @@ def test_plain_fields() -> None: assert completion_item.display_meta_plain == "A tasty apple" +def test_sanitization() -> None: + """Test display string sanitization 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.""" From 19c36d63136a6968f98965499e757a031cac6b96 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Wed, 1 Apr 2026 15:52:15 -0400 Subject: [PATCH 2/5] Compiling regular expression at the class level. --- cmd2/completion.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/cmd2/completion.py b/cmd2/completion.py index d368d982f..127b22d4d 100644 --- a/cmd2/completion.py +++ b/cmd2/completion.py @@ -51,6 +51,8 @@ class CompletionItem: """A single completion result.""" + _SANITIZE_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,14 +78,14 @@ class CompletionItem: display_plain: str = field(init=False) display_meta_plain: str = field(init=False) - @staticmethod - def _sanitize_display_string(val: str) -> str: + @classmethod + def _sanitize_display_string(cls, val: str) -> str: """Sanitize a string for display in the completion menu. This replaces whitespace characters that are rendered as control sequences (like ^J or ^I) with spaces. """ - return re.sub(r'\r\n|[\n\r\t\f\v]', ' ', val) + return cls._SANITIZE_RE.sub(' ', val) def __post_init__(self) -> None: """Finalize the object after initialization.""" From 52c06fa7cc7b24d41210947df5c5cbad6899d12e Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Wed, 1 Apr 2026 16:18:56 -0400 Subject: [PATCH 3/5] Made sanitize_display_string a module-level utility function. --- cmd2/completion.py | 37 +++++++++++++++++++++---------------- 1 file changed, 21 insertions(+), 16 deletions(-) diff --git a/cmd2/completion.py b/cmd2/completion.py index 127b22d4d..d17f7e594 100644 --- a/cmd2/completion.py +++ b/cmd2/completion.py @@ -31,8 +31,8 @@ from . import rich_utils as ru -# Regular expression to identify strings which we should sort numerically -NUMERIC_RE = re.compile( +# Regular expression to identify strings that we should sort numerically +_NUMERIC_RE = re.compile( r""" ^ # Start of string [-+]? # Optional sign @@ -46,13 +46,15 @@ re.VERBOSE, ) +# 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]') + @dataclass(frozen=True, slots=True, kw_only=True) class CompletionItem: """A single completion result.""" - _SANITIZE_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) @@ -78,15 +80,6 @@ class CompletionItem: display_plain: str = field(init=False) display_meta_plain: str = field(init=False) - @classmethod - def _sanitize_display_string(cls, val: str) -> str: - """Sanitize a string for display in the completion menu. - - This replaces whitespace characters that are rendered as - control sequences (like ^J or ^I) with spaces. - """ - return cls._SANITIZE_RE.sub(' ', val) - def __post_init__(self) -> None: """Finalize the object after initialization.""" # Derive text from value if it wasn't explicitly provided @@ -98,8 +91,8 @@ def __post_init__(self) -> None: object.__setattr__(self, "display", self.text) # Sanitize display and display_meta - object.__setattr__(self, "display", self._sanitize_display_string(self.display)) - object.__setattr__(self, "display_meta", self._sanitize_display_string(self.display_meta)) + object.__setattr__(self, "display", sanitize_display_string(self.display)) + object.__setattr__(self, "display_meta", sanitize_display_string(self.display_meta)) # Create plain text versions by stripping ANSI sequences. # These are stored as attributes for fast access during sorting/filtering. @@ -268,4 +261,16 @@ class Completions(CompletionResultsBase): 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) + return bool(items) and all(_NUMERIC_RE.match(item.display_plain) for item in items) + + +def sanitize_display_string(val: str) -> str: + """Sanitize 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 sanitized + :return: the sanitized string + """ + return _CONTROL_WHITESPACE_RE.sub(' ', val) From 1d185f7298aba1560599de60752e233a9c27aa04 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Wed, 1 Apr 2026 23:22:00 -0400 Subject: [PATCH 4/5] Simplified code with encapsulation. --- cmd2/argparse_completer.py | 12 +++--- cmd2/completion.py | 85 ++++++++++++++++++++------------------ tests/test_completion.py | 12 +++--- 3 files changed, 56 insertions(+), 53 deletions(-) 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 d17f7e594..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,30 +30,15 @@ from . import rich_utils as ru -# 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, -) - -# 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]') - @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) @@ -80,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 @@ -90,9 +86,9 @@ def __post_init__(self) -> None: if not self.display: object.__setattr__(self, "display", self.text) - # Sanitize display and display_meta - object.__setattr__(self, "display", sanitize_display_string(self.display)) - object.__setattr__(self, "display_meta", sanitize_display_string(self.display_meta)) + # 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. @@ -143,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) @@ -150,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: @@ -257,20 +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) - - -def sanitize_display_string(val: str) -> str: - """Sanitize 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 sanitized - :return: the sanitized string - """ - return _CONTROL_WHITESPACE_RE.sub(' ', val) diff --git a/tests/test_completion.py b/tests/test_completion.py index 99b925190..366364044 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: From 288deec9e1b7fd5606cac4d9acfc37a9786e8ba8 Mon Sep 17 00:00:00 2001 From: Kevin Van Brunt Date: Wed, 1 Apr 2026 23:36:57 -0400 Subject: [PATCH 5/5] Updated test function. --- tests/test_completion.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_completion.py b/tests/test_completion.py index 366364044..02018ba3c 100644 --- a/tests/test_completion.py +++ b/tests/test_completion.py @@ -930,8 +930,8 @@ def test_plain_fields() -> None: assert completion_item.display_meta_plain == "A tasty apple" -def test_sanitization() -> None: - """Test display string sanitization in CompletionItem.""" +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"