[GitHub] [airflow] hamdanal commented on a diff in pull request #29116: Add colors in help outputs of Airfow CLI commands #28789

["hamdanal (via GitHub)" <gi...@apache.org>]

hamdanal commented on code in PR #29116:
URL: https://github.com/apache/airflow/pull/29116#discussion_r1093696005


##########
airflow/cli/cli_parser.py:
##########
@@ -2201,46 +2202,56 @@ def _remove_dag_id_opt(command: ActionCommand):
 DAG_CLI_DICT: dict[str, CLICommand] = {sp.name: sp for sp in dag_cli_commands}
 
 
-class AirflowHelpFormatter(argparse.HelpFormatter):
+class AirflowHelpFormatter(RichHelpFormatter):
     """
     Custom help formatter to display help message.
 
     It displays simple commands and groups of commands in separate sections.
     """
 
-    def _format_action(self, action: Action):
+    def _rich_format_action(self, action: Action):

Review Comment:
   Awesome to see `rich-argparse` work in such an amazing project. Please let me know if you find any bugs or quirks, I'll be happy to address them.
   
   The TLDR answer to your question is, I really doubt the interface of `_rich_format_action` will change a lot but it is a private method so stability cannot be promised.
   
   ### Long answer
   `_rich_format_action` and some other `RichHelpFormatter` methods with similar names are based on the same methods in `argparse.HelpFormatter` without the `_rich` prefix. Since the `argparse` methods are private, the equivalent `_rich...` methods are also private. Have the original methods been public, I would've made the `_rich` methods public. The point is I have no intention or interest in introducing a breaking change in these methods however if the interfaces of the original `argparse` methods change, the `_rich` methods will follow that change. I think this is a very unlikely scenario because `argparse` is currently maintained in this "extremely stable mode" and rarely gets any updates (even bug fixes take years to be accepted if ever). That said I don't like to give you a promise I can't honor, hence the short answer above.
   
   ### Possible solution
   From what I understand reading the implementation of this method here, you seem to be interested in two things (please correct me if I am wrong):
   1. Sort the subcommands into two sections: Groups and Commands
   2. Print a section title at the top of each section
   
   Both of these are possible using the vanilla `argparse.HelpFormatter` method [`_iter_indented_subactions`](https://github.com/python/cpython/blob/main/Lib/argparse.py#L635-L643) without needing to change any `rich-argparse` specific code. The trick is to create an empty "fake" action with `nargs=0` that prints the section header before its content. This way you get something that works with both the original formatter and the `rich-argparse` formatter. I think this will be even simpler implementation compared to your current solution.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org