diff --git a/cmd2/argparse_completer.py b/cmd2/argparse_completer.py index 208153f1f..c2643b60c 100644 --- a/cmd2/argparse_completer.py +++ b/cmd2/argparse_completer.py @@ -18,6 +18,8 @@ cast, ) +from rich.text import Text + from .constants import INFINITY from .rich_utils import Cmd2GeneralConsole @@ -587,7 +589,7 @@ def _complete_flags(self, text: str, line: str, begidx: int, endidx: int, used_f return Completions(items) def _format_completions(self, arg_state: _ArgumentState, completions: Completions) -> Completions: - """Format CompletionItems into hint table.""" + """Format CompletionItems into completion table.""" # Skip table generation for single results or if the list exceeds the # user-defined threshold for table display. if len(completions) < 2 or len(completions) > self._cmd2_app.max_completion_table_items: @@ -611,7 +613,7 @@ def _format_completions(self, arg_state: _ArgumentState, completions: Completion # Determine if all display values are numeric so we can right-align them all_nums = all_display_numeric(completions.items) - # Build header row for the hint table + # Build header row rich_columns: list[Column] = [] rich_columns.append(Column(destination.upper(), justify="right" if all_nums else "left", no_wrap=True)) table_header = cast(Sequence[str | Column] | None, arg_state.action.get_table_header()) # type: ignore[attr-defined] @@ -621,12 +623,12 @@ def _format_completions(self, arg_state: _ArgumentState, completions: Completion column if isinstance(column, Column) else Column(column, overflow="fold") for column in table_header ) - # Build the hint table + # Add the data rows hint_table = Table(*rich_columns, box=SIMPLE_HEAD, show_edge=False, border_style=Cmd2Style.TABLE_BORDER) for item in completions: - hint_table.add_row(item.display, *item.table_row) + hint_table.add_row(Text.from_ansi(item.display), *item.table_row) - # Generate the hint table string + # Generate the table string console = Cmd2GeneralConsole() with console.capture() as capture: console.print(hint_table, end="", soft_wrap=False) diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py index a03f937c3..1cd11cb6e 100644 --- a/cmd2/cmd2.py +++ b/cmd2/cmd2.py @@ -1189,12 +1189,20 @@ def allow_style_type(value: str) -> ru.AllowStyle: f"must be {ru.AllowStyle.ALWAYS}, {ru.AllowStyle.NEVER}, or {ru.AllowStyle.TERMINAL} (case-insensitive)" ) from ex + settable_description = Text.assemble( + 'Allow styled text in output (Options: ', + (str(ru.AllowStyle.ALWAYS), Style(bold=True)), + ", ", + (str(ru.AllowStyle.NEVER), Style(bold=True)), + ", ", + (str(ru.AllowStyle.TERMINAL), Style(bold=True)), + ")", + ) self.add_settable( Settable( 'allow_style', allow_style_type, - 'Allow ANSI text style sequences in output (valid values: ' - f'{ru.AllowStyle.ALWAYS}, {ru.AllowStyle.NEVER}, {ru.AllowStyle.TERMINAL})', + ru.rich_text_to_string(settable_description), self, choices_provider=get_allow_style_choices, ) @@ -1211,7 +1219,7 @@ def allow_style_type(value: str) -> ru.AllowStyle: Settable( 'max_completion_table_items', int, - "Maximum number of completion results allowed for a completion table to appear", + "Max results allowed to display a table", self, ) ) @@ -1219,7 +1227,7 @@ def allow_style_type(value: str) -> ru.AllowStyle: Settable( 'max_column_completion_results', int, - "Maximum number of completion results to display in a single column", + "Max results to display in a single column", self, ) ) @@ -2496,11 +2504,13 @@ def _get_settable_choices(self) -> Choices: items: list[CompletionItem] = [] for name, settable in self.settables.items(): + value_str = str(settable.value) table_row = [ - str(settable.value), + value_str, settable.description, ] - items.append(CompletionItem(name, display_meta=str(settable.value), table_row=table_row)) + display_meta = f"[Current: {su.stylize(value_str, Style(bold=True))}] {settable.description}" + items.append(CompletionItem(name, display_meta=display_meta, table_row=table_row)) return Choices(items=items) @@ -4414,7 +4424,7 @@ def do_set(self, args: argparse.Namespace) -> None: settable_table.add_row( param, str(settable.value), - settable.description, + Text.from_ansi(settable.description), ) self.last_result[param] = settable.value diff --git a/cmd2/completion.py b/cmd2/completion.py index 671df48cb..d6e1afe93 100644 --- a/cmd2/completion.py +++ b/cmd2/completion.py @@ -21,6 +21,8 @@ overload, ) +from . import string_utils as su + if TYPE_CHECKING: # pragma: no cover from .cmd2 import Cmd from .command_definition import CommandSet @@ -64,15 +66,22 @@ class CompletionItem: text: str = "" # Optional string for displaying the completion differently in the completion menu. + # This can contain ANSI style sequences. A plain version is stored in display_plain. display: str = "" # Optional meta information about completion which displays in the completion menu. + # This can contain ANSI style sequences. A plain version is stored in display_meta_plain. display_meta: str = "" # Optional row data for completion tables. Length must match the associated argparse # argument's table_header. This is stored internally as a tuple. table_row: Sequence[Any] = field(default_factory=tuple) + # Plain text versions of display fields (stripped of ANSI) for sorting/filtering. + # These are set in __post_init__(). + display_plain: str = field(init=False) + display_meta_plain: str = field(init=False) + def __post_init__(self) -> None: """Finalize the object after initialization.""" # Derive text from value if it wasn't explicitly provided @@ -83,6 +92,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. + # 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)) + # Make sure all table row objects are renderable by a Rich table. renderable_data = [obj if is_renderable(obj) else str(obj) for obj in self.table_row] @@ -140,10 +154,10 @@ def __post_init__(self) -> None: if not self.is_sorted: if all_display_numeric(unique_items): # Sort numerically - unique_items.sort(key=lambda item: float(item.display)) + unique_items.sort(key=lambda item: float(item.display_plain)) else: # Standard string sort - unique_items.sort(key=lambda item: utils.DEFAULT_STR_SORT_KEY(item.display)) + unique_items.sort(key=lambda item: utils.DEFAULT_STR_SORT_KEY(item.display_plain)) object.__setattr__(self, "is_sorted", True) @@ -247,8 +261,8 @@ class Completions(CompletionResultsBase): def all_display_numeric(items: Collection[CompletionItem]) -> bool: - """Return True if items is non-empty and every item.display is a numeric string.""" - return bool(items) and all(NUMERIC_RE.match(item.display) for item in items) + """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/cmd2/pt_utils.py b/cmd2/pt_utils.py index 75ff47d45..a79afa14d 100644 --- a/cmd2/pt_utils.py +++ b/cmd2/pt_utils.py @@ -24,6 +24,7 @@ constants, utils, ) +from . import rich_utils as ru if TYPE_CHECKING: # pragma: no cover from .cmd2 import Cmd @@ -101,6 +102,9 @@ def get_completions(self, document: Document, _complete_event: object) -> Iterab buffer.cursor_right(search_text_length) return + # Determine if we should remove style from completion text + remove_style = ru.ALLOW_STYLE == ru.AllowStyle.NEVER + # Return the completions for item in completions: # Set offset to the start of the current word to overwrite it with the completion @@ -129,8 +133,8 @@ def get_completions(self, document: Document, _complete_event: object) -> Iterab yield Completion( match_text, start_position=start_position, - display=item.display, - display_meta=item.display_meta, + display=item.display_plain if remove_style else ANSI(item.display), + display_meta=item.display_meta_plain if remove_style else ANSI(item.display_meta), ) diff --git a/tests/scripts/postcmds.txt b/tests/scripts/postcmds.txt index 30f470550..7f93a5d46 100644 --- a/tests/scripts/postcmds.txt +++ b/tests/scripts/postcmds.txt @@ -1 +1 @@ -set allow_style Never +set always_show_hint False diff --git a/tests/scripts/precmds.txt b/tests/scripts/precmds.txt index 7d036acfe..241504ff4 100644 --- a/tests/scripts/precmds.txt +++ b/tests/scripts/precmds.txt @@ -1 +1 @@ -set allow_style Always +set always_show_hint True diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py index d5256661f..0d5165eb9 100644 --- a/tests/test_cmd2.py +++ b/tests/test_cmd2.py @@ -465,11 +465,11 @@ def test_run_script_nested_run_scripts(base_app, request) -> None: expected = f""" {initial_run} _relative_run_script precmds.txt -set allow_style Always +set always_show_hint True help shortcuts _relative_run_script postcmds.txt -set allow_style Never""" +set always_show_hint False""" out, _err = run_cmd(base_app, 'history -s') assert out == normalize(expected) @@ -482,11 +482,11 @@ def test_runcmds_plus_hooks(base_app, request) -> None: base_app.runcmds_plus_hooks(['run_script ' + prefilepath, 'help', 'shortcuts', 'run_script ' + postfilepath]) expected = f""" run_script {prefilepath} -set allow_style Always +set always_show_hint True help shortcuts run_script {postfilepath} -set allow_style Never""" +set always_show_hint False""" out, _err = run_cmd(base_app, 'history -s') assert out == normalize(expected) @@ -2349,9 +2349,9 @@ def test_get_settable_choices(base_app: cmd2.Cmd) -> None: assert cur_settable is not None str_value = str(cur_settable.value) - assert cur_choice.display_meta == str_value - assert cur_choice.table_row[0] == str_value - assert cur_choice.table_row[1] == cur_settable.description + assert str_value in cur_choice.display_meta + assert ru.rich_text_to_string(cur_choice.table_row[0]) == str_value + assert ru.rich_text_to_string(cur_choice.table_row[1]) == cur_settable.description def test_completion_supported(base_app) -> None: diff --git a/tests/test_completion.py b/tests/test_completion.py index 0436abaf7..a17ce6a59 100644 --- a/tests/test_completion.py +++ b/tests/test_completion.py @@ -19,6 +19,7 @@ Completions, utils, ) +from cmd2.completion import all_display_numeric from .conftest import ( normalize, @@ -877,6 +878,125 @@ def test_completions_iteration() -> None: assert list(reversed(completions)) == items[::-1] +def test_numeric_sorting() -> None: + """Test that numbers and numeric strings are sorted numerically.""" + numbers = [5, 6, 4, 3, 7.2, 9.1] + completions = Completions.from_values(numbers) + assert [item.value for item in completions] == sorted(numbers) + + number_strs = ["5", "6", "4", "3", "7.2", "9.1"] + completions = Completions.from_values(number_strs) + assert list(completions.to_strings()) == sorted(number_strs, key=float) + + mixed = ["5", "6", "4", 3, "7.2", 9.1] + completions = Completions.from_values(mixed) + assert list(completions.to_strings()) == [str(v) for v in sorted(number_strs, key=float)] + + +def test_is_sorted() -> None: + """Test that already sorted results are not re-sorted.""" + values = [5, 6, 4, 3] + already_sorted = Completions.from_values(values, is_sorted=True) + sorted_on_creation = Completions.from_values(values, is_sorted=False) + + assert already_sorted.to_strings() != sorted_on_creation.to_strings() + assert [item.value for item in already_sorted] == values + + +@pytest.mark.parametrize( + ('values', 'all_nums'), + [ + ([2, 3], True), + ([2, 3.7], True), + ([2, "3"], True), + ([2.2, "3.4"], True), + ([2, "3g"], False), + # The display_plain field strips off ANSI sequences + (["\x1b[31m5\x1b[0m", "\x1b[32m9.2\x1b[0m"], True), + (["\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_remove_duplicates() -> None: + """Test that duplicate CompletionItems are removed.""" + + # Create items which alter the fields used in CompletionItem.__eq__(). + orig_item = CompletionItem(value="orig item", display="orig display", display_meta="orig meta") + new_value = dataclasses.replace(orig_item, value="new value") + new_text = dataclasses.replace(orig_item, text="new text") + new_display = dataclasses.replace(orig_item, display="new display") + new_meta = dataclasses.replace(orig_item, display_meta="new meta") + + # Include each item twice. + items = [orig_item, orig_item, new_value, new_value, new_text, new_text, new_display, new_display, new_meta, new_meta] + completions = Completions(items) + + # Make sure we have exactly 1 of each item. + assert len(completions) == 5 + assert orig_item in completions + assert new_value in completions + assert new_text in completions + assert new_display in completions + assert new_meta in completions + + +def test_plain_fields() -> None: + """Test the plain text fields in CompletionItem.""" + display = "\x1b[31mApple\x1b[0m" + display_meta = "\x1b[32mA tasty apple\x1b[0m" + + # Show that the plain fields remove the ANSI sequences. + completion_item = CompletionItem("apple", display=display, display_meta=display_meta) + assert completion_item.display == display + assert completion_item.display_plain == "Apple" + assert completion_item.display_meta == display_meta + assert completion_item.display_meta_plain == "A tasty apple" + + +def test_styled_completion_sort() -> None: + """Test that sorting is done with the display_plain field.""" + + # First sort with strings that include ANSI style sequences. + red_apple = "\x1b[31mApple\x1b[0m" + green_cherry = "\x1b[32mCherry\x1b[0m" + blue_banana = "\x1b[34mBanana\x1b[0m" + + # This sorts by ASCII: [31m (Red), [32m (Green), [34m (Blue) + unsorted_strs = [blue_banana, red_apple, green_cherry] + sorted_strs = sorted(unsorted_strs, key=utils.DEFAULT_STR_SORT_KEY) + assert sorted_strs == [red_apple, green_cherry, blue_banana] + + # Now create a Completions object with these values. + unsorted_items = [ + CompletionItem("banana", display=blue_banana), + CompletionItem("cherry", display=green_cherry), + CompletionItem("apple", display=red_apple), + ] + + completions = Completions(unsorted_items) + + # Expected order: Apple (A), Banana (B), Cherry (C) + expected_plain = ["Apple", "Banana", "Cherry"] + expected_styled = [red_apple, blue_banana, green_cherry] + + for index, item in enumerate(completions): + # Prove the ANSI stripping worked correctly + assert item.display_plain == expected_plain[index] + + # Prove the sort order used the plain text, not the ANSI codes + assert item.display == expected_styled[index] + + # Prove the order of completions is not the same as the raw string sort order + completion_displays = [item.display for item in completions] + assert completion_displays != sorted_strs + + # Used by redirect_complete tests class RedirCompType(enum.Enum): SHELL_CMD = (1,) diff --git a/tests/test_pt_utils.py b/tests/test_pt_utils.py index 78a2d3480..99d2f990f 100644 --- a/tests/test_pt_utils.py +++ b/tests/test_pt_utils.py @@ -7,9 +7,18 @@ import pytest from prompt_toolkit.buffer import Buffer from prompt_toolkit.document import Document +from prompt_toolkit.formatted_text import ( + ANSI, + to_formatted_text, +) import cmd2 -from cmd2 import pt_utils, utils +from cmd2 import ( + Cmd2Style, + pt_utils, + stylize, + utils, +) from cmd2.history import HistoryItem from cmd2.parsing import Statement @@ -191,10 +200,19 @@ def test_get_completions(self, mock_cmd_app: MockCmd, monkeypatch) -> None: line = "" document = Document(line, cursor_position=0) + # Test plain and styled values for display and display_meta + foo_text = "foo" + foo_display = "Foo Display" + foo_meta = "Foo Meta" + + bar_text = "bar" + bar_display = stylize("Bar Display", Cmd2Style.SUCCESS) + bar_meta = stylize("Bar Meta", Cmd2Style.WARNING) + # Set up matches completion_items = [ - cmd2.CompletionItem("foo", display="Foo Display"), - cmd2.CompletionItem("bar", display="Bar Display"), + cmd2.CompletionItem(foo_text, display=foo_display, display_meta=foo_meta), + cmd2.CompletionItem(bar_text, display=bar_display, display_meta=bar_meta), ] cmd2_completions = cmd2.Completions(completion_items, completion_table="Table Data") mock_cmd_app.complete.return_value = cmd2_completions @@ -202,13 +220,15 @@ def test_get_completions(self, mock_cmd_app: MockCmd, monkeypatch) -> None: # Call get_completions completions = list(completer.get_completions(document, None)) - # Verify completions which are sorted by display field. assert len(completions) == len(cmd2_completions) - assert completions[0].text == "bar" - assert completions[0].display == [('', 'Bar Display')] - assert completions[1].text == "foo" - assert completions[1].display == [('', 'Foo Display')] + assert completions[0].text == bar_text + assert to_formatted_text(completions[0].display) == to_formatted_text(ANSI(bar_display)) + assert to_formatted_text(completions[0].display_meta) == to_formatted_text(ANSI(bar_meta)) + + assert completions[1].text == foo_text + assert to_formatted_text(completions[1].display) == to_formatted_text(ANSI(foo_display)) + assert to_formatted_text(completions[1].display_meta) == to_formatted_text(ANSI(foo_meta)) # Verify that only the completion table printed assert mock_print.call_count == 1