diff --git a/CHANGELOG.md b/CHANGELOG.md index 8747a4df..8fc003e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,14 @@ ## 2.3.1 (TBD, 2021) +* Bug Fixes + * Fixed issue introduced in 2.3.0 with `AlternatingTable`, `BorderedTable`, and `SimpleTable` that caused + header alignment settings to be overridden by data alignment settings. * Enhancements - * Added ability to use `CompletionItems` as argparse choices + * `CompletionItems` now saves the original object from which it creates a string. + * Using `CompletionItems` as argparse choices is fully supported. `cmd2` patched `argparse` to compare input to + the original value instead of the `CompletionItems` instance. + * `ArgparseCompleter` now does the following if a list of `CompletionItems` was created with numerical types + * Sorts completion hints numerically + * Right-aligns the left-most column in completion hint table ## 2.3.0 (November 11, 2021) * Bug Fixes @@ -13,7 +21,7 @@ and/or data text. This allows for things like nesting an AlternatingTable in another AlternatingTable. * AlternatingTable no longer automatically applies background color to borders. This was done to improve appearance since the background color extended beyond the borders of the table. - * Added ability to colorize all aspects of `AlternatingTables`, `BorderedTables`, and `SimpleTables`. + * Added ability to colorize all aspects of `AlternatingTable`, `BorderedTable`, and `SimpleTable`. * Added support for 8-bit/256-colors with the `cmd2.EightBitFg` and `cmd2.EightBitBg` classes. * Added support for 24-bit/RGB colors with the `cmd2.RgbFg` and `cmd2.RgbBg` classes. * Removed dependency on colorama. diff --git a/cmd2/argparse_completer.py b/cmd2/argparse_completer.py index 26835513..9ec6677a 100644 --- a/cmd2/argparse_completer.py +++ b/cmd2/argparse_completer.py @@ -49,6 +49,7 @@ ) from .table_creator import ( Column, + HorizontalAlignment, SimpleTable, ) @@ -547,15 +548,32 @@ def _complete_flags(self, text: str, line: str, begidx: int, endidx: int, matche return matches def _format_completions(self, arg_state: _ArgumentState, completions: Union[List[str], List[CompletionItem]]) -> List[str]: - # Check if the results are CompletionItems and that there aren't too many to display - if 1 < len(completions) <= self._cmd2_app.max_completion_items and isinstance(completions[0], CompletionItem): - completion_items = cast(List[CompletionItem], completions) - four_spaces = 4 * ' ' + """Format CompletionItems into hint table""" + + # Nothing to do if we don't have at least 2 completions which are all CompletionItems + if len(completions) < 2 or not all(isinstance(c, CompletionItem) for c in completions): + return cast(List[str], completions) + + completion_items = cast(List[CompletionItem], completions) - # If the user has not already sorted the CompletionItems, then sort them before appending the descriptions - if not self._cmd2_app.matches_sorted: + # Check if the data being completed have a numerical type + all_nums = all(isinstance(c.orig_value, numbers.Number) for c in completion_items) + + # Sort CompletionItems before building the hint table + if not self._cmd2_app.matches_sorted: + # If all orig_value types are numbers, then sort by that value + if all_nums: + completion_items.sort(key=lambda c: c.orig_value) # type: ignore[no-any-return] + + # Otherwise sort as strings + else: completion_items.sort(key=self._cmd2_app.default_sort_key) - self._cmd2_app.matches_sorted = True + + self._cmd2_app.matches_sorted = True + + # Check if there are too many CompletionItems to display as a table + if len(completions) <= self._cmd2_app.max_completion_items: + four_spaces = 4 * ' ' # If a metavar was defined, use that instead of the dest field destination = arg_state.action.metavar if arg_state.action.metavar else arg_state.action.dest @@ -588,13 +606,22 @@ def _format_completions(self, arg_state: _ArgumentState, completions: Union[List desc_width = max(widest_line(item.description), desc_width) cols = list() - cols.append(Column(destination.upper(), width=token_width)) + dest_alignment = HorizontalAlignment.RIGHT if all_nums else HorizontalAlignment.LEFT + cols.append( + Column( + destination.upper(), + width=token_width, + header_horiz_align=dest_alignment, + data_horiz_align=dest_alignment, + ) + ) cols.append(Column(desc_header, width=desc_width)) hint_table = SimpleTable(cols, divider_char=self._cmd2_app.ruler) table_data = [[item, item.description] for item in completion_items] self._cmd2_app.formatted_completions = hint_table.generate_table(table_data, row_spacing=0) + # Return sorted list of completions return cast(List[str], completions) def complete_subcommand_help(self, text: str, line: str, begidx: int, endidx: int, tokens: List[str]) -> List[str]: diff --git a/cmd2/argparse_custom.py b/cmd2/argparse_custom.py index 871f9d25..80e405f3 100644 --- a/cmd2/argparse_custom.py +++ b/cmd2/argparse_custom.py @@ -332,12 +332,12 @@ def __init__(self, value: object, description: str = '', *args: Any) -> None: # Save the original value to support CompletionItems as argparse choices. # cmd2 has patched argparse so input is compared to this value instead of the CompletionItem instance. - self.__orig_value = value + self._orig_value = value @property def orig_value(self) -> Any: - """Read-only property for __orig_value""" - return self.__orig_value + """Read-only property for _orig_value""" + return self._orig_value ############################################################################################################ diff --git a/cmd2/table_creator.py b/cmd2/table_creator.py index 19eead49..1e9755c7 100644 --- a/cmd2/table_creator.py +++ b/cmd2/table_creator.py @@ -416,8 +416,9 @@ def _generate_cell_lines(self, cell_data: Any, is_header: bool, col: Column, fil def generate_row( self, + row_data: Sequence[Any], + is_header: bool, *, - row_data: Optional[Sequence[Any]] = None, fill_char: str = SPACE, pre_line: str = EMPTY, inter_cell: str = (2 * SPACE), @@ -426,8 +427,9 @@ def generate_row( """ Generate a header or data table row - :param row_data: If this is None then a header row is generated. Otherwise data should have an entry for each - column in the row. (Defaults to None) + :param row_data: data with an entry for each column in the row + :param is_header: True if writing a header cell, otherwise writing a data cell. This determines whether to + use header or data alignment settings defined in the Columns. :param fill_char: character that fills remaining space in a cell. Defaults to space. If this is a tab, then it will be converted to one space. (Cannot be a line breaking character) :param pre_line: string to print before each line of a row. This can be used for a left row border and @@ -453,13 +455,8 @@ def __init__(self) -> None: # Display width of this cell self.width = 0 - if row_data is None: - row_data = [col.header for col in self.cols] - is_header = True - else: - if len(row_data) != len(self.cols): - raise ValueError("Length of row_data must match length of cols") - is_header = False + if len(row_data) != len(self.cols): + raise ValueError("Length of row_data must match length of cols") # Replace tabs (tabs in data strings will be handled in _generate_cell_lines()) fill_char = fill_char.replace('\t', SPACE) @@ -654,14 +651,14 @@ def generate_header(self) -> str: # Apply background color to header text in Columns which allow it to_display: List[Any] = [] - for index, col in enumerate(self.cols): + for col in self.cols: if col.style_header_text: to_display.append(self.apply_header_bg(col.header)) else: to_display.append(col.header) # Create the header labels - header_labels = self.generate_row(row_data=to_display, fill_char=fill_char, inter_cell=inter_cell) + header_labels = self.generate_row(to_display, is_header=True, fill_char=fill_char, inter_cell=inter_cell) header_buf.write(header_labels) # Add the divider if necessary @@ -696,7 +693,7 @@ def generate_data_row(self, row_data: Sequence[Any]) -> str: else: to_display.append(row_data[index]) - return self.generate_row(row_data=to_display, fill_char=fill_char, inter_cell=inter_cell) + return self.generate_row(to_display, is_header=False, fill_char=fill_char, inter_cell=inter_cell) def generate_table(self, table_data: Sequence[Sequence[Any]], *, include_header: bool = True, row_spacing: int = 1) -> str: """ @@ -855,7 +852,8 @@ def generate_table_top_border(self) -> str: post_line = self.padding * '═' + '╗' return self.generate_row( - row_data=self.empty_data, + self.empty_data, + is_header=False, fill_char=self.apply_border_color(fill_char), pre_line=self.apply_border_color(pre_line), inter_cell=self.apply_border_color(inter_cell), @@ -876,7 +874,8 @@ def generate_header_bottom_border(self) -> str: post_line = self.padding * '═' + '╣' return self.generate_row( - row_data=self.empty_data, + self.empty_data, + is_header=False, fill_char=self.apply_border_color(fill_char), pre_line=self.apply_border_color(pre_line), inter_cell=self.apply_border_color(inter_cell), @@ -898,7 +897,8 @@ def generate_row_bottom_border(self) -> str: post_line = self.padding * '─' + '╢' return self.generate_row( - row_data=self.empty_data, + self.empty_data, + is_header=False, fill_char=self.apply_border_color(fill_char), pre_line=self.apply_border_color(pre_line), inter_cell=self.apply_border_color(inter_cell), @@ -919,7 +919,8 @@ def generate_table_bottom_border(self) -> str: post_line = self.padding * '═' + '╝' return self.generate_row( - row_data=self.empty_data, + self.empty_data, + is_header=False, fill_char=self.apply_border_color(fill_char), pre_line=self.apply_border_color(pre_line), inter_cell=self.apply_border_color(inter_cell), @@ -941,7 +942,7 @@ def generate_header(self) -> str: # Apply background color to header text in Columns which allow it to_display: List[Any] = [] - for index, col in enumerate(self.cols): + for col in self.cols: if col.style_header_text: to_display.append(self.apply_header_bg(col.header)) else: @@ -953,7 +954,7 @@ def generate_header(self) -> str: header_buf.write('\n') header_buf.write( self.generate_row( - row_data=to_display, fill_char=fill_char, pre_line=pre_line, inter_cell=inter_cell, post_line=post_line + to_display, is_header=True, fill_char=fill_char, pre_line=pre_line, inter_cell=inter_cell, post_line=post_line ) ) header_buf.write('\n') @@ -988,7 +989,7 @@ def generate_data_row(self, row_data: Sequence[Any]) -> str: to_display.append(row_data[index]) return self.generate_row( - row_data=to_display, fill_char=fill_char, pre_line=pre_line, inter_cell=inter_cell, post_line=post_line + to_display, is_header=False, fill_char=fill_char, pre_line=pre_line, inter_cell=inter_cell, post_line=post_line ) def generate_table(self, table_data: Sequence[Sequence[Any]], *, include_header: bool = True) -> str: diff --git a/tests/test_argparse_completer.py b/tests/test_argparse_completer.py index 9b5e1b31..3c9da6bf 100644 --- a/tests/test_argparse_completer.py +++ b/tests/test_argparse_completer.py @@ -115,12 +115,15 @@ def do_pos_and_flag(self, args: argparse.Namespace) -> None: CUSTOM_DESC_HEADER = "Custom Header" # Lists used in our tests (there is a mix of sorted and unsorted on purpose) - non_negative_int_choices = [1, 2, 3, 0, 22] - int_choices = [-1, 1, -2, 2, 0, -12] + non_negative_num_choices = [1, 2, 3, 0.5, 22] + num_choices = [-1, 1, -2, 2.5, 0, -12] static_choices_list = ['static', 'choices', 'stop', 'here'] choices_from_provider = ['choices', 'provider', 'probably', 'improved'] completion_item_choices = [CompletionItem('choice_1', 'A description'), CompletionItem('choice_2', 'Another description')] + # This tests that CompletionItems created with numerical values are sorted as numbers. + num_completion_items = [CompletionItem(5, "Five"), CompletionItem(1.5, "One.Five"), CompletionItem(2, "Five")] + def choices_provider(self) -> List[str]: """Method that provides choices""" return self.choices_from_provider @@ -141,14 +144,12 @@ def completion_item_method(self) -> List[CompletionItem]: "-p", "--provider", help="a flag populated with a choices provider", choices_provider=choices_provider ) choices_parser.add_argument( - '-d', "--desc_header", help='this arg has a descriptive header', choices_provider=completion_item_method, descriptive_header=CUSTOM_DESC_HEADER, ) choices_parser.add_argument( - '-n', "--no_header", help='this arg has no descriptive header', choices_provider=completion_item_method, @@ -162,8 +163,11 @@ def completion_item_method(self) -> List[CompletionItem]: metavar=TUPLE_METAVAR, nargs=argparse.ONE_OR_MORE, ) - choices_parser.add_argument('-i', '--int', type=int, help='a flag with an int type', choices=int_choices) + 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 + ) # Positional args for choices command choices_parser.add_argument("list_pos", help="a positional populated with a choices list", choices=static_choices_list) @@ -171,7 +175,7 @@ def completion_item_method(self) -> List[CompletionItem]: "method_pos", help="a positional populated with a choices provider", choices_provider=choices_provider ) choices_parser.add_argument( - 'non_negative_int', type=int, help='a positional with non-negative int choices', choices=non_negative_int_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=[]) @@ -558,10 +562,11 @@ def test_autcomp_flag_completion(ac_app, command_and_args, text, completion_matc ('--list', 's', ['static', 'stop']), ('-p', '', ArgparseCompleterTester.choices_from_provider), ('--provider', 'pr', ['provider', 'probably']), - ('-i', '', ArgparseCompleterTester.int_choices), - ('--int', '1', ['1 ']), - ('--int', '-', [-1, -2, -12]), - ('--int', '-1', [-1, -12]), + ('-n', '', ArgparseCompleterTester.num_choices), + ('--num', '1', ['1 ']), + ('--num', '-', [-1, -2, -12]), + ('--num', '-1', [-1, -12]), + ('--num_completion_items', '', ArgparseCompleterTester.num_completion_items), ], ) def test_autocomp_flag_choices_completion(ac_app, flag, text, completions): @@ -592,7 +597,7 @@ def test_autocomp_flag_choices_completion(ac_app, flag, text, completions): (1, 's', ['static', 'stop']), (2, '', ArgparseCompleterTester.choices_from_provider), (2, 'pr', ['provider', 'probably']), - (3, '', ArgparseCompleterTester.non_negative_int_choices), + (3, '', ArgparseCompleterTester.non_negative_num_choices), (3, '2', [2, 22]), (4, '', []), ], diff --git a/tests/test_table_creator.py b/tests/test_table_creator.py index bc6909f8..52c4da8b 100644 --- a/tests/test_table_creator.py +++ b/tests/test_table_creator.py @@ -91,24 +91,24 @@ def test_column_alignment(): width=10, header_horiz_align=HorizontalAlignment.LEFT, header_vert_align=VerticalAlignment.TOP, - data_horiz_align=HorizontalAlignment.LEFT, - data_vert_align=VerticalAlignment.TOP, + data_horiz_align=HorizontalAlignment.RIGHT, + data_vert_align=VerticalAlignment.BOTTOM, ) column_2 = Column( "Col 2", width=10, - header_horiz_align=HorizontalAlignment.CENTER, - header_vert_align=VerticalAlignment.MIDDLE, + header_horiz_align=HorizontalAlignment.RIGHT, + header_vert_align=VerticalAlignment.BOTTOM, data_horiz_align=HorizontalAlignment.CENTER, data_vert_align=VerticalAlignment.MIDDLE, ) column_3 = Column( "Col 3", width=10, - header_horiz_align=HorizontalAlignment.RIGHT, - header_vert_align=VerticalAlignment.BOTTOM, - data_horiz_align=HorizontalAlignment.RIGHT, - data_vert_align=VerticalAlignment.BOTTOM, + header_horiz_align=HorizontalAlignment.CENTER, + header_vert_align=VerticalAlignment.MIDDLE, + data_horiz_align=HorizontalAlignment.LEFT, + data_vert_align=VerticalAlignment.TOP, ) column_4 = Column("Three\nline\nheader", width=10) @@ -122,20 +122,21 @@ def test_column_alignment(): assert column_4.data_vert_align == VerticalAlignment.TOP # Create a header row - header = tc.generate_row() + row_data = [col.header for col in columns] + header = tc.generate_row(row_data=row_data, is_header=True) assert header == ( 'Col 1 Three \n' - ' Col 2 line \n' - ' Col 3 header ' + ' Col 3 line \n' + ' Col 2 header ' ) # Create a data row row_data = ["Val 1", "Val 2", "Val 3", "Three\nline\ndata"] - row = tc.generate_row(row_data=row_data) + row = tc.generate_row(row_data=row_data, is_header=False) assert row == ( - 'Val 1 Three \n' + ' Val 3 Three \n' ' Val 2 line \n' - ' Val 3 data ' + ' Val 1 data ' ) @@ -145,16 +146,16 @@ def test_blank_last_line(): tc = TableCreator([column_1]) row_data = ['my line\n\n'] - row = tc.generate_row(row_data=row_data) + row = tc.generate_row(row_data=row_data, is_header=False) assert row == ('my line \n' ' ') row_data = ['\n'] - row = tc.generate_row(row_data=row_data) + row = tc.generate_row(row_data=row_data, is_header=False) assert row == ' ' row_data = [''] - row = tc.generate_row(row_data=row_data) + row = tc.generate_row(row_data=row_data, is_header=False) assert row == ' ' @@ -164,7 +165,7 @@ def test_wrap_text(): # Test normal wrapping row_data = ['Some text to wrap\nA new line that will wrap\nNot wrap\n 1 2 3'] - row = tc.generate_row(row_data=row_data) + row = tc.generate_row(row_data=row_data, is_header=False) assert row == ('Some text \n' 'to wrap \n' 'A new line\n' @@ -175,7 +176,7 @@ def test_wrap_text(): # Test preserving a multiple space sequence across a line break row_data = ['First last one'] - row = tc.generate_row(row_data=row_data) + row = tc.generate_row(row_data=row_data, is_header=False) assert row == ('First \n' ' last one ') @@ -186,31 +187,31 @@ def test_wrap_text_max_lines(): # Test not needing to truncate the final line row_data = ['First line last line'] - row = tc.generate_row(row_data=row_data) + row = tc.generate_row(row_data=row_data, is_header=False) assert row == ('First line\n' 'last line ') # Test having to truncate the last word because it's too long for the final line row_data = ['First line last lineextratext'] - row = tc.generate_row(row_data=row_data) + row = tc.generate_row(row_data=row_data, is_header=False) assert row == ('First line\n' 'last line…') # Test having to truncate the last word because it fits the final line but there is more text not being included row_data = ['First line thistxtfit extra'] - row = tc.generate_row(row_data=row_data) + row = tc.generate_row(row_data=row_data, is_header=False) assert row == ('First line\n' 'thistxtfi…') # Test having to truncate the last word because it fits the final line but there are more lines not being included row_data = ['First line thistxtfit\nextra'] - row = tc.generate_row(row_data=row_data) + row = tc.generate_row(row_data=row_data, is_header=False) assert row == ('First line\n' 'thistxtfi…') # Test having space left on the final line and adding an ellipsis because there are more lines not being included row_data = ['First line last line\nextra line'] - row = tc.generate_row(row_data=row_data) + row = tc.generate_row(row_data=row_data, is_header=False) assert row == ('First line\n' 'last line…') @@ -224,7 +225,8 @@ def test_wrap_long_word(): tc = TableCreator(columns) # Test header row - header = tc.generate_row() + row_data = [col.header for col in columns] + header = tc.generate_row(row_data, is_header=True) assert header == ('LongColumn \n' 'Name Col 2 ') @@ -237,7 +239,7 @@ def test_wrap_long_word(): # Long word should start on the second line row_data.append("Word LongerThan10") - row = tc.generate_row(row_data=row_data) + row = tc.generate_row(row_data=row_data, is_header=False) expected = ( TextStyle.RESET_ALL + Fg.GREEN @@ -281,7 +283,7 @@ def test_wrap_long_word_max_data_lines(): # This long word will start on the final line after another word. Therefore it won't wrap but will instead be truncated. row_data.append("A LongerThan10RunsOverLast") - row = tc.generate_row(row_data=row_data) + row = tc.generate_row(row_data=row_data, is_header=False) assert row == ('LongerThan LongerThan LongerThan A LongerT…\n' '10FitsLast 10FitsLas… 10RunsOve… ') @@ -293,7 +295,7 @@ def test_wrap_long_char_wider_than_max_width(): """ column_1 = Column("Col 1", width=1) tc = TableCreator([column_1]) - row = tc.generate_row(row_data=['深']) + row = tc.generate_row(row_data=['深'], is_header=False) assert row == '…' @@ -304,29 +306,31 @@ def test_generate_row_exceptions(): # fill_char too long with pytest.raises(TypeError) as excinfo: - tc.generate_row(row_data=row_data, fill_char='too long') + tc.generate_row(row_data=row_data, is_header=False, fill_char='too long') assert "Fill character must be exactly one character long" in str(excinfo.value) # Unprintable characters for arg in ['fill_char', 'pre_line', 'inter_cell', 'post_line']: kwargs = {arg: '\n'} with pytest.raises(ValueError) as excinfo: - tc.generate_row(row_data=row_data, **kwargs) + tc.generate_row(row_data=row_data, is_header=False, **kwargs) assert "{} contains an unprintable character".format(arg) in str(excinfo.value) # data with too many columns row_data = ['Data 1', 'Extra Column'] with pytest.raises(ValueError) as excinfo: - tc.generate_row(row_data=row_data) + tc.generate_row(row_data=row_data, is_header=False) assert "Length of row_data must match length of cols" in str(excinfo.value) def test_tabs(): column_1 = Column("Col\t1", width=20) column_2 = Column("Col 2") - tc = TableCreator([column_1, column_2], tab_width=2) + columns = [column_1, column_2] + tc = TableCreator(columns, tab_width=2) - row = tc.generate_row(fill_char='\t', pre_line='\t', inter_cell='\t', post_line='\t') + row_data = [col.header for col in columns] + row = tc.generate_row(row_data, is_header=True, fill_char='\t', pre_line='\t', inter_cell='\t', post_line='\t') assert row == ' Col 1 Col 2 ' with pytest.raises(ValueError) as excinfo: