Removed DEFAULT_TABLE_HEADER.

Author: kmvanbrunt

CHANGELOG.md

@@ -39,6 +39,8 @@ prompt is displayed.
     - `choices_provider` functions must now return a `cmd2.Choices` object instead of `list[str]`.
     - An argparse argument's `descriptive_headers` field is now called `table_header`.
     - `CompletionItem.descriptive_data` is now called `CompletionItem.table_row`.
+    - Removed `DEFAULT_DESCRIPTIVE_HEADERS`. This means you must define `table_header` when using
+      `CompletionItem.table_row` data.
     - `Cmd.default_sort_key` moved to `utils.DEFAULT_STR_SORT_KEY`.
     - Moved completion state data, which previously resided in `Cmd`, into other classes.
         - `Cmd.matches_sorted` -> `Completions.is_sorted` and `Choices.is_sorted`

cmd2/argparse_completer.py

@@ -48,9 +48,6 @@
 from .exceptions import CompletionError
 from .styles import Cmd2Style
 
-# If no table header is supplied, then this will be used instead
-DEFAULT_TABLE_HEADER: Sequence[str | Column] = ['Description']
-
 # Name of the choice/completer function argument that, if present, will be passed a dictionary of
 # command line tokens up through the token being completed mapped to their argparse destination name.
 ARG_TOKENS = 'arg_tokens'
@@ -591,15 +588,48 @@ def _complete_flags(self, text: str, line: str, begidx: int, endidx: int, used_f
 
         return Completions(items)
 
+    @staticmethod
+    def _validate_table_data(arg_state: _ArgumentState, completions: Completions) -> None:
+        """Verify the integrity of completion table data.
+
+        :raises ValueError: if there is an error with the data.
+        """
+        table_header = arg_state.action.get_table_header()  # type: ignore[attr-defined]
+        has_table_data = any(item.table_row for item in completions)
+
+        if table_header is None:
+            if has_table_data:
+                raise ValueError(
+                    f"Argument '{arg_state.action.dest}' has CompletionItems with table_row, "
+                    f"but no table_header was defined in add_argument()."
+                )
+            return
+
+        # If header is defined, then every item must have data, and lengths must match
+        for item in completions:
+            if not item.table_row:
+                raise ValueError(
+                    f"Argument '{arg_state.action.dest}' has table_header defined, "
+                    f"but the CompletionItem for '{item.text}' is missing table_row."
+                )
+            if len(item.table_row) != len(table_header):
+                raise ValueError(
+                    f"Argument '{arg_state.action.dest}': table_row length ({len(item.table_row)}) "
+                    f"does not match table_header length ({len(table_header)}) for item '{item.text}'."
+                )
+
     def _build_completion_table(self, arg_state: _ArgumentState, completions: Completions) -> Completions:
         """Build a rich.Table for completion results if applicable."""
-        # 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:
-            return completions
+        # Verify integrity of completion data
+        self._validate_table_data(arg_state, completions)
 
-        # Ensure every item provides table metadata to avoid an incomplete table.
-        if not all(item.table_row for item in completions):
+        table_header = cast(
+            Sequence[str | Column] | None,
+            arg_state.action.get_table_header(),  # type: ignore[attr-defined]
+        )
+
+        # Skip table generation if results are outside thresholds or no columns are defined
+        if len(completions) < 2 or len(completions) > self._cmd2_app.max_completion_table_items or table_header is None:
             return completions
 
         # If a metavar was defined, use that instead of the dest field
@@ -619,9 +649,6 @@ def _build_completion_table(self, arg_state: _ArgumentState, completions: Comple
         # 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]
-        if table_header is None:
-            table_header = DEFAULT_TABLE_HEADER
         rich_columns.extend(
             column if isinstance(column, Column) else Column(column, overflow="fold") for column in table_header
         )

tests/test_argparse_completer.py

@@ -105,7 +105,7 @@ def do_pos_and_flag(self, args: argparse.Namespace) -> None:
     ############################################################################################################
     STR_METAVAR = "HEADLESS"
     TUPLE_METAVAR = ('arg1', 'others')
-    CUSTOM_TABLE_HEADER = ("Custom Header",)
+    DESCRIPTION_TABLE_HEADER = ("Description",)
 
     # tuples (for sake of immutability) used in our tests (there is a mix of sorted and unsorted on purpose)
     non_negative_num_choices = (1, 2, 3, 0.5, 22)
@@ -140,45 +140,82 @@ def completion_item_method(self) -> list[CompletionItem]:
     choices_parser = Cmd2ArgumentParser()
 
     # Flag args for choices command. Include string and non-string arg types.
-    choices_parser.add_argument("-l", "--list", help="a flag populated with a choices list", choices=static_choices_list)
     choices_parser.add_argument(
-        "-p", "--provider", help="a flag populated with a choices provider", choices_provider=choices_provider
+        "-l",
+        "--list",
+        help="a flag populated with a choices list",
+        choices=static_choices_list,
     )
     choices_parser.add_argument(
-        "--table_header",
-        help='this arg has a table header',
+        "-p",
+        "--provider",
+        help="a flag populated with a choices provider",
+        choices_provider=choices_provider,
+    )
+    choices_parser.add_argument(
+        "--no_metavar",
+        help='this arg has no metavar',
         choices_provider=completion_item_method,
-        table_header=CUSTOM_TABLE_HEADER,
+        table_header=DESCRIPTION_TABLE_HEADER,
     )
     choices_parser.add_argument(
-        "--no_header",
-        help='this arg has no table header',
+        "--str_metavar",
+        help='this arg has str for a metavar',
         choices_provider=completion_item_method,
         metavar=STR_METAVAR,
+        table_header=DESCRIPTION_TABLE_HEADER,
     )
     choices_parser.add_argument(
         '-t',
         "--tuple_metavar",
         help='this arg has tuple for a metavar',
-        choices_provider=completion_item_method,
         metavar=TUPLE_METAVAR,
         nargs=argparse.ONE_OR_MORE,
+        choices_provider=completion_item_method,
+        table_header=DESCRIPTION_TABLE_HEADER,
+    )
+    choices_parser.add_argument(
+        '-n',
+        '--num',
+        type=int,
+        help='a flag with an int type',
+        choices=num_choices,
+    )
+    choices_parser.add_argument(
+        '--completion_items',
+        help='choices are CompletionItems',
+        choices=completion_item_choices,
+        table_header=DESCRIPTION_TABLE_HEADER,
     )
-    choices_parser.add_argument('-n', '--num', type=int, help='a flag with an int type', choices=num_choices)
-    choices_parser.add_argument('--completion_items', help='choices are CompletionItems', choices=completion_item_choices)
     choices_parser.add_argument(
-        '--num_completion_items', help='choices are numerical CompletionItems', choices=num_completion_items
+        '--num_completion_items',
+        help='choices are numerical CompletionItems',
+        choices=num_completion_items,
+        table_header=DESCRIPTION_TABLE_HEADER,
     )
 
     # Positional args for choices command
-    choices_parser.add_argument("list_pos", help="a positional populated with a choices list", choices=static_choices_list)
     choices_parser.add_argument(
-        "method_pos", help="a positional populated with a choices provider", choices_provider=choices_provider
+        "list_pos",
+        help="a positional populated with a choices list",
+        choices=static_choices_list,
+    )
+    choices_parser.add_argument(
+        "method_pos",
+        help="a positional populated with a choices provider",
+        choices_provider=choices_provider,
     )
     choices_parser.add_argument(
-        'non_negative_num', type=int, help='a positional with non-negative numerical choices', choices=non_negative_num_choices
+        'non_negative_num',
+        type=int,
+        help='a positional with non-negative numerical choices',
+        choices=non_negative_num_choices,
+    )
+    choices_parser.add_argument(
+        'empty_choices',
+        help='a positional with empty choices',
+        choices=[],
     )
-    choices_parser.add_argument('empty_choices', help='a positional with empty choices', choices=[])
 
     @with_argparser(choices_parser)
     def do_choices(self, args: argparse.Namespace) -> None:
@@ -854,20 +891,20 @@ def test_unfinished_flag_error(ac_app, command_and_args, text, is_error) -> None
     assert is_error == all(x in completions.error for x in ["Error: argument", "expected"])
 
 
-def test_completion_table_arg_header(ac_app) -> None:
+def test_completion_table_metavar(ac_app) -> None:
     # Test when metavar is None
     text = ''
-    line = f'choices --table_header {text}'
+    line = f'choices --no_metavar {text}'
     endidx = len(line)
     begidx = endidx - len(text)
 
     completions = ac_app.complete(text, line, begidx, endidx)
     assert completions.table is not None
-    assert completions.table.columns[0].header == "TABLE_HEADER"
+    assert completions.table.columns[0].header == "NO_METAVAR"
 
     # Test when metavar is a string
     text = ''
-    line = f'choices --no_header {text}'
+    line = f'choices --str_metavar {text}'
     endidx = len(line)
     begidx = endidx - len(text)
 
@@ -908,32 +945,6 @@ def test_completion_table_arg_header(ac_app) -> None:
     assert completions.table.columns[0].header == ac_app.TUPLE_METAVAR[1].upper()
 
 
-def test_completion_table_header(ac_app) -> None:
-    from cmd2.argparse_completer import (
-        DEFAULT_TABLE_HEADER,
-    )
-
-    # This argument provided a table header
-    text = ''
-    line = f'choices --table_header {text}'
-    endidx = len(line)
-    begidx = endidx - len(text)
-
-    completions = ac_app.complete(text, line, begidx, endidx)
-    assert completions.table is not None
-    assert ac_app.CUSTOM_TABLE_HEADER[0] == completions.table.columns[1].header
-
-    # This argument did not provide a table header, so it should be DEFAULT_TABLE_HEADER
-    text = ''
-    line = f'choices --no_header {text}'
-    endidx = len(line)
-    begidx = endidx - len(text)
-
-    completions = ac_app.complete(text, line, begidx, endidx)
-    assert completions.table is not None
-    assert DEFAULT_TABLE_HEADER[0] == completions.table.columns[1].header
-
-
 @pytest.mark.parametrize(
     ('command_and_args', 'text', 'has_hint'),
     [
@@ -1165,6 +1176,94 @@ def test_display_meta(ac_app, subcommand, flag, display_meta) -> None:
     assert completions[0].display_meta == display_meta
 
 
+def test_validate_table_data_no_table() -> None:
+    action = argparse.Action(option_strings=['-f'], dest='foo')
+    action.set_table_header(None)
+    arg_state = argparse_completer._ArgumentState(action)
+    completions = Completions(
+        [
+            CompletionItem('item1'),
+            CompletionItem('item2'),
+        ]
+    )
+
+    # This should not raise an exception
+    argparse_completer.ArgparseCompleter._validate_table_data(arg_state, completions)
+
+
+def test_validate_table_data_missing_header() -> None:
+    action = argparse.Action(option_strings=['-f'], dest='foo')
+    action.set_table_header(None)
+    arg_state = argparse_completer._ArgumentState(action)
+
+    completions = Completions(
+        [
+            CompletionItem('item1', table_row=['data1']),
+            CompletionItem('item2', table_row=['data2']),
+        ]
+    )
+
+    with pytest.raises(
+        ValueError,
+        match="Argument 'foo' has CompletionItems with table_row, but no table_header was defined",
+    ):
+        argparse_completer.ArgparseCompleter._validate_table_data(arg_state, completions)
+
+
+def test_validate_table_data_missing_row_data() -> None:
+    action = argparse.Action(option_strings=['-f'], dest='foo')
+    action.set_table_header(['Col1'])
+    arg_state = argparse_completer._ArgumentState(action)
+
+    completions = Completions(
+        [
+            CompletionItem('item1', table_row=['data1']),
+            CompletionItem('item2'),  # Missing table_row
+        ]
+    )
+
+    with pytest.raises(
+        ValueError,
+        match="Argument 'foo' has table_header defined, but the CompletionItem for 'item2' is missing table_row",
+    ):
+        argparse_completer.ArgparseCompleter._validate_table_data(arg_state, completions)
+
+
+def test_validate_table_row_data_length_mismatch() -> None:
+    action = argparse.Action(option_strings=['-f'], dest='foo')
+    action.set_table_header(['Col1', 'Col2'])
+    arg_state = argparse_completer._ArgumentState(action)
+
+    completions = Completions(
+        [
+            CompletionItem('item1', table_row=['data1a', 'data1b']),
+            CompletionItem('item2', table_row=['only_one']),
+        ]
+    )
+
+    with pytest.raises(
+        ValueError,
+        match=r"Argument 'foo': table_row length \(1\) does not match table_header length \(2\) for item 'item2'.",
+    ):
+        argparse_completer.ArgparseCompleter._validate_table_data(arg_state, completions)
+
+
+def test_validate_table_data_valid() -> None:
+    action = argparse.Action(option_strings=['-f'], dest='foo')
+    action.get_table_header = lambda: ['Col1', 'Col2']
+    arg_state = argparse_completer._ArgumentState(action)
+
+    completions = Completions(
+        [
+            CompletionItem('item1', table_row=['data1a', 'data1b']),
+            CompletionItem('item2', table_row=['data2a', 'data2b']),
+        ]
+    )
+
+    # This should not raise an exception
+    argparse_completer.ArgparseCompleter._validate_table_data(arg_state, completions)
+
+
 # Custom ArgparseCompleter-based class
 class CustomCompleter(argparse_completer.ArgparseCompleter):
     def _complete_flags(self, text: str, line: str, begidx: int, endidx: int, matched_flags: list[str]) -> list[str]: