Skip to content

Simplified class-wide command categorization#1626

Merged
kmvanbrunt merged 11 commits intomainfrom
simplify_categorization
Apr 8, 2026
Merged

Simplified class-wide command categorization#1626
kmvanbrunt merged 11 commits intomainfrom
simplify_categorization

Conversation

@kmvanbrunt
Copy link
Copy Markdown
Member

Simplified class-wide command categorization by introducing the DEFAULT_CATEGORY class variable to Cmd and CommandSet.

@kmvanbrunt kmvanbrunt requested a review from tleonhardt as a code owner April 7, 2026 20:15
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 7, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.51%. Comparing base (6fbf084) to head (27acf59).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1626      +/-   ##
==========================================
- Coverage   99.59%   99.51%   -0.09%     
==========================================
  Files          21       21              
  Lines        4736     4709      -27     
==========================================
- Hits         4717     4686      -31     
- Misses         19       23       +4     
Flag Coverage Δ
unittests 99.51% <100.00%> (-0.09%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Copy Markdown
Member

@tleonhardt tleonhardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A Gemini CLI code review said this. Let me know what you think.

File: cmd2/cmd2.py
  L870: [HIGH] Removing automatic categorization breaks the enabling and disabling of command categories.

  Because constants.CMD_ATTR_HELP_CATEGORY is no longer automatically applied to commands that inherit their category via DEFAULT_CATEGORY, the subsequent check for self.disabled_categories will fail to disable these
  commands upon registration. This same bug breaks enable_category() and disable_category() (around L5625 and L5687), as they also strictly rely on CMD_ATTR_HELP_CATEGORY to identify a command's category. Consider
  extracting the category resolution logic from _build_command_info into a shared helper method (e.g., _get_command_category(func)) to ensure consistency across the codebase.

  Suggested change:

    1                  self._cmd_to_command_sets[command] = cmdset
    2  
    3 -                if default_category and not hasattr(command_method, constants.CMD_ATTR_HELP_CATEGORY):
    4 -                    utils.categorize(command_method, default_category)
    5 -
    6 -                # If this command is in a disabled category, then disable it
    7 -                command_category = getattr(command_method, constants.CMD_ATTR_HELP_CATEGORY, None)
    8 -                if command_category in self.disabled_categories:
    9 -                    message_to_print = self.disabled_categories[command_category]
   10 -                    self.disable_command(command, message_to_print)
   11 +                # Determine command category for disabling
   12 +                if hasattr(command_method, constants.CMD_ATTR_HELP_CATEGORY):
   13 +                    command_category = getattr(command_method, constants.CMD_ATTR_HELP_CATEGORY)
   14 +                elif command_method.__doc__ or cmd_help is not None or hasattr(command_method, constants.CMD_ATTR_ARGPARSER):
   15 +                    defining_cls = get_defining_class(command_method) or cmdset.__class__
   16 +                    command_category = getattr(defining_cls, 'DEFAULT_CATEGORY', self.DEFAULT_CATEGORY)
   17 +                else:
   18 +                    command_category = None
   19 +
   20 +                # If this command is in a disabled category, then disable it
   21 +                if command_category in self.disabled_categories:
   22 +                    message_to_print = self.disabled_categories[command_category]
   23 +                    self.disable_command(command, message_to_print)

…EGORY.

Removed concept of undocumented commands.
tleonhardt
tleonhardt previously approved these changes Apr 8, 2026
@kmvanbrunt kmvanbrunt merged commit 1d6910a into main Apr 8, 2026
32 of 33 checks passed
@kmvanbrunt kmvanbrunt deleted the simplify_categorization branch April 8, 2026 01:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants