Implemented custom help messaging rather than relying on argparse

wordfence/cli/cli.py

@@ -9,30 +9,44 @@
 from ..scanning.scanner import ExceptionContainer
 from .banner.banner import show_welcome_banner_if_enabled
 from .config import load_config, RenamedSubcommandException
+from .config.base_config_definitions import config_map \
+        as base_config_map
 from .subcommands import load_subcommand_definitions
 from .context import CliContext
 from .configurer import Configurer
 from .terms import TermsManager
+from .helper import Helper
 
 
 class WordfenceCli:
 
     def __init__(self):
         self.initialize_early_logging()
         self.subcommand_definitions = load_subcommand_definitions()
+        self.helper = self._initialize_helper()
+        self._load_config()
+        self.initialize_logging(self.config.verbose)
+        self.cache = self.initialize_cache()
+        self.subcommand = None
+
+    def _initialize_helper(self) -> Helper:
+        return Helper(
+                self.subcommand_definitions,
+                base_config_map
+            )
+
+    def _load_config(self) -> None:
         try:
             self.config, self.subcommand_definition = load_config(
                     self.subcommand_definitions,
+                    self.helper
                 )
         except RenamedSubcommandException as rename:
             print(
                     f'The "{rename.old}" subcommand has been renamed to '
                     f'"{rename.new}"'
                 )
             sys.exit(1)
-        self.initialize_logging(self.config.verbose)
-        self.cache = self.initialize_cache()
-        self.subcommand = None
 
     def print_error(self, message: str) -> None:
         if sys.stderr is not None:
@@ -94,15 +108,23 @@ def process_exception(self, exception: BaseException) -> int:
                 self.print_error(message)
         return 1
 
+    def display_help(self) -> None:
+        self.helper.display_help(self.config.subcommand)
+
     def invoke(self) -> int:
         if self.config.purge_cache:
             self.cache.purge()
 
         show_welcome_banner_if_enabled(self.config)
 
+        if self.config.help:
+            self.display_help()
+            return 0
+
         context = CliContext(
                 self.config,
-                self.cache
+                self.cache,
+                self.helper
             )
 
         if self.config.version:
@@ -124,7 +146,7 @@ def invoke(self) -> int:
         context.configurer = configurer
 
         if self.subcommand_definition is None:
-            self.config.display_help()
+            self.display_help()
             configurer.check_config()
             return 0
 

wordfence/cli/config/__init__.py

@@ -1,5 +1,6 @@
 from typing import List, Dict, Tuple
 
+from ..helper import Helper
 from .cli_parser import CliCanonicalValueExtractor, get_cli_values
 from .config_items import ConfigItemDefinition, \
     CanonicalValueExtractorInterface, not_set_token
@@ -79,10 +80,12 @@ def _get_renamed_subcommand(
 
 def load_config(
             subcommand_definitions: Dict[str, SubcommandDefinition],
+            helper: Helper,
             subcommand: str = None
         ) -> Tuple[Config, SubcommandDefinition]:
     cli_values, trailing_arguments, parser = get_cli_values(
-            subcommand_definitions
+            subcommand_definitions,
+            helper
         )
 
     if subcommand is None:

wordfence/cli/config/base_config_definitions.py

@@ -15,7 +15,8 @@
                        "running in a TTY/terminal.",
         "context": "ALL",
         "argument_type": "FLAG",
-        "default": True
+        "default": True,
+        "category": "Output Control"
     },
     "license": {
         "short_name": "l",
@@ -31,46 +32,59 @@
                        "TTY. Use --no-verbose to disable.",
         "context": "ALL",
         "argument_type": "OPTIONAL_FLAG",
-        "default": None
+        "default": None,
+        "category": "Logging"
     },
     "debug": {
         "short_name": "d",
         "description": "Enable debug logging.",
         "context": "ALL",
         "argument_type": "FLAG",
-        "default": False
+        "default": False,
+        "category": "Logging"
     },
     "quiet": {
         "short_name": "q",
         "description": "Suppress all output other than scan results.",
         "context": "ALL",
         "argument_type": "FLAG",
-        "default": False
+        "default": False,
+        "category": "Logging"
     },
     "cache-directory": {
         "description": "A path to use for cache files.",
         "context": "ALL",
         "argument_type": "OPTION",
-        "default": "~/.cache/wordfence"
+        "default": "~/.cache/wordfence",
+        "category": "Caching"
     },
     "cache": {
         "description": "Whether or not to enable the cache.",
         "context": "ALL",
         "argument_type": "FLAG",
-        "default": True
+        "default": True,
+        "category": "Caching"
     },
     "purge-cache": {
         "description": "Purge any existing values from the cache.",
         "context": "CLI",
         "argument_type": "FLAG",
-        "default": False
+        "default": False,
+        "category": "Caching"
     },
     "version": {
         "description": "Display the version of Wordfence CLI.",
         "context": "CLI",
         "argument_type": "FLAG",
         "default": False
     },
+    "help": {
+        "short_name": "h",
+        "description": "Display help information",
+        "context": "CLI",
+        "argument_type": "FLAG",
+        "default": False
+    },
     # Hidden options
     "check-for-update": {
         "description": "Whether or not to run the update check.",

wordfence/cli/config/cli_parser.py

@@ -4,6 +4,7 @@
 from typing import Set, List, Dict, Any, Tuple
 
 from wordfence.logging import log
+from ..helper import Helper
 from .config_items import ConfigItemDefinition, \
     CanonicalValueExtractorInterface, Context, ArgumentType, \
     not_set_token
@@ -73,12 +74,6 @@ def add_to_parser(target_parser,
             f"Config value {json.dumps(config_definition.name)} is not a valid"
             f" CLI argument. Should it be specified in the INI file instead?")
         return
-    if config_definition.name == 'help' or config_definition.short_name == 'h':
-        # change this behavior by setting
-        # ArgumentParser kwarg add_help to False
-        raise ValueError(
-            "A help command cannot be defined, as one is added automatically"
-            " by the parser")
 
     names: List[str] = [f"--{config_definition.name}"]
     if config_definition.short_name:
@@ -122,19 +117,15 @@ def add_to_parser(target_parser,
             not named_params['action'].startswith('store_'):
         named_params['type'] = config_definition.get_value_type()
 
-    if config_definition.hidden:
-        named_params['help'] = argparse.SUPPRESS
+    named_params['help'] = argparse.SUPPRESS
 
     target_parser.add_argument(*names, **named_params)
 
     # register the negation of a flag
     if config_definition.is_flag():
         named_params['action'] = 'store_false'
         names = [f"--no-{config_definition.name}"]
-        if config_definition.hidden or not config_definition.default:
-            named_params['help'] = argparse.SUPPRESS
-        else:
-            del named_params['help']
+        named_params['help'] = argparse.SUPPRESS
         # set the value to override the un-prefixed command
         named_params['dest'] = config_definition.property_name
         target_parser.add_argument(*names, **named_params)
@@ -149,24 +140,28 @@ def add_definitions_to_parser(
 
 
 def get_cli_values(
-            subcommand_definitions: Dict[str, SubcommandDefinition]
+            subcommand_definitions: Dict[str, SubcommandDefinition],
+            helper: Helper
         ) -> Tuple[Namespace, List[str], ArgumentParser]:
     parser = ArgumentParser(
             prog=COMMAND,
-            description=DESCRIPTION
+            description=DESCRIPTION,
+            add_help=False,
+            usage=helper.generate_usage()
         )
 
     add_definitions_to_parser(parser, base_config_map)
 
-    subparsers = parser.add_subparsers(title="Wordfence CLI subcommands",
+    subparsers = parser.add_subparsers(title="Available Subcommands",
                                        dest="subcommand",
                                        metavar='')
     for subcommand_definition in subcommand_definitions.values():
         definitions = subcommand_definition.get_config_map()
         subparser = subparsers.add_parser(
                 subcommand_definition.name,
                 prog=subcommand_definition.name,
-                help=subcommand_definition.description
+                add_help=False,
+                usage=helper.generate_usage(),
             )
         add_definitions_to_parser(subparser, base_config_map)
         add_definitions_to_parser(subparser, definitions)

wordfence/cli/config/config_items.py

@@ -67,6 +67,7 @@ class ConfigItemDefinition:
     hidden: bool = False
     short_name: Optional[str] = None
     meta: Optional[ConfigItemMeta] = None
+    category: str = 'General Options'
 
     @staticmethod
     def clean_argument_dict(source: Dict[str, Any]) -> Dict[str, Any]:

wordfence/cli/configure/definition.py

@@ -7,6 +7,7 @@
 
 definition = SubcommandDefinition(
     name='configure',
+    usage='',
     description='Interactively configure Wordfence CLI',
     config_definitions=config_definitions,
     config_section='DEFAULT',

wordfence/cli/context.py

@@ -10,10 +10,11 @@
 
 class CliContext:
 
-    def __init__(self, config: Config, cache: Cache):
+    def __init__(self, config: Config, cache: Cache, helper):
         self.config = config
         cache.add_filter(self.filter_cache_entry)
         self.cache = cache
+        self.helper = helper
         self._license = None
         self._noc1_client = None
         self._terms_update_hooks = []

wordfence/cli/help/definition.py

@@ -7,6 +7,7 @@
 
 definition = SubcommandDefinition(
     name='help',
+    usage='[SUBCOMMAND]',
     description='Display help',
     config_definitions=config_definitions,
     config_section='DEFAULT',

wordfence/cli/help/help.py

@@ -4,7 +4,12 @@
 class HelpSubcommand(Subcommand):
 
     def invoke(self) -> int:
-        self.config.display_help()
+        subcommand = None
+        for argument in self.config.trailing_arguments:
+            if subcommand is not None:
+                raise Exception('Please specify a single subcommand')
+            subcommand = argument
+        self.helper.display_help(subcommand)
         return 0