[PATCH net-next v2 0/7] tools: ynl: cli: improve the help and doc

@@ -20,6 +20,29 @@ from lib import YnlFamily, Netlink, NlError, SpecFamily, SpecException, YnlExcep
 SYS_SCHEMA_DIR='/usr/share/ynl'
 RELATIVE_SCHEMA_DIR='../../../../Documentation/netlink'
 
+# pylint: disable=too-few-public-methods,too-many-locals
+class Colors:
+    """ANSI color and font modifier codes"""
+    RESET = '\033[0m'
+
+    BOLD = '\033[1m'
+    ITALICS = '\033[3m'
+    UNDERLINE = '\033[4m'
+    INVERT = '\033[7m'
+
+
+def color(text, modifiers):
+    """Add color to text if output is a TTY
+
+    Returns:
+        Colored text if stdout is a TTY, otherwise plain text
+    """
+    if sys.stdout.isatty():
+        # Join the colors if they are a list, if it's a string this a noop
+        modifiers = "".join(modifiers)
+        return f"{modifiers}{text}{Colors.RESET}"
+    return text
+
 def schema_dir():
     """
     Return the effective schema directory, preferring in-tree before

@@ -60,7 +83,7 @@ RELATIVE_SCHEMA_DIR='../../../../Documentation/netlink'
     for attr_name in attr_names:
         if attr_name in attr_set.attrs:
             attr = attr_set.attrs[attr_name]
-            attr_info = f'{prefix}- {attr_name}: {attr.type}'
+            attr_info = f'{prefix}- {color(attr_name, Colors.BOLD)}: {attr.type}'
             if 'enum' in attr.yaml:
                 enum_name = attr.yaml['enum']
                 attr_info += f" (enum: {enum_name})"

@@ -68,7 +91,8 @@ RELATIVE_SCHEMA_DIR='../../../../Documentation/netlink'
                 if enum_name in ynl.consts:
                     const = ynl.consts[enum_name]
                     enum_values = list(const.entries.keys())
-                    attr_info += f"\n{prefix}  {const.type.capitalize()}: {', '.join(enum_values)}"
+                    type_fmted = color(const.type.capitalize(), Colors.ITALICS)
+                    attr_info += f"\n{prefix}  {type_fmted}: {', '.join(enum_values)}"
 
             # Show nested attributes reference and recursively display them
             nested_set_name = None

@@ -226,7 +250,7 @@ RELATIVE_SCHEMA_DIR='../../../../Documentation/netlink'
             print(f'Operation {args.list_attrs} not found')
             sys.exit(1)
 
-        print(f'Operation: {op.name}')
+        print(f'Operation: {color(op.name, Colors.BOLD)}')
         print(op.yaml['doc'])
 
         for mode in ['do', 'dump', 'event']:

@@ -10,6 +10,7 @@ import json
 import os
 import pathlib
 import pprint
+import shutil
 import sys
 import textwrap
 

@@ -101,7 +102,11 @@ RELATIVE_SCHEMA_DIR='../../../../Documentation/netlink'
                 attr_info += f" -> {nested_set_name}"
 
             if attr.yaml.get('doc'):
-                doc_text = textwrap.indent(attr.yaml['doc'], prefix + '  ')
+                doc_prefix = prefix + ' ' * 4
+                term_width = shutil.get_terminal_size().columns
+                doc_text = textwrap.fill(attr.yaml['doc'], width=term_width,
+                                         initial_indent=doc_prefix,
+                                         subsequent_indent=doc_prefix)
                 attr_info += f"\n{doc_text}"
             print(attr_info)
 

@@ -151,47 +151,78 @@ RELATIVE_SCHEMA_DIR='../../../../Documentation/netlink'
     """
 
     parser = argparse.ArgumentParser(description=description,
-                                     epilog=epilog)
-    spec_group = parser.add_mutually_exclusive_group(required=True)
-    spec_group.add_argument('--family', dest='family', type=str,
-                            help='name of the netlink FAMILY')
-    spec_group.add_argument('--list-families', action='store_true',
-                            help='list all netlink families supported by YNL (has spec)')
-    spec_group.add_argument('--spec', dest='spec', type=str,
-                            help='choose the family by SPEC file path')
+                                     epilog=epilog, add_help=False)
 
-    parser.add_argument('--schema', dest='schema', type=str)
-    parser.add_argument('--no-schema', action='store_true')
-    parser.add_argument('--json', dest='json_text', type=str)
+    gen_group = parser.add_argument_group('General options')
+    gen_group.add_argument('-h', '--help', action='help',
+                           help='show this help message and exit')
 
-    group = parser.add_mutually_exclusive_group()
-    group.add_argument('--do', dest='do', metavar='DO-OPERATION', type=str)
-    group.add_argument('--multi', dest='multi', nargs=2, action='append',
-                       metavar=('DO-OPERATION', 'JSON_TEXT'), type=str)
-    group.add_argument('--dump', dest='dump', metavar='DUMP-OPERATION', type=str)
-    group.add_argument('--list-ops', action='store_true')
-    group.add_argument('--list-msgs', action='store_true')
-    group.add_argument('--list-attrs', dest='list_attrs', metavar='OPERATION', type=str,
-                       help='List attributes for an operation')
-    group.add_argument('--validate', action='store_true')
+    spec_group = parser.add_argument_group('Netlink family selection')
+    spec_sel = spec_group.add_mutually_exclusive_group(required=True)
+    spec_sel.add_argument('--list-families', action='store_true',
+                          help=('list Netlink families supported by YNL '
+                                '(which have a spec available in the standard '
+                                'system path)'))
+    spec_sel.add_argument('--family', dest='family', type=str,
+                          help='name of the Netlink FAMILY to use')
+    spec_sel.add_argument('--spec', dest='spec', type=str,
+                          help='full file path to the YAML spec file')
+
+    ops_group = parser.add_argument_group('Operations')
+    ops = ops_group.add_mutually_exclusive_group()
+    ops.add_argument('--do', dest='do', metavar='DO-OPERATION', type=str)
+    ops.add_argument('--dump', dest='dump', metavar='DUMP-OPERATION', type=str)
+    ops.add_argument('--multi', dest='multi', nargs=2, action='append',
+                     metavar=('DO-OPERATION', 'JSON_TEXT'), type=str,
+                     help="Multi-message operation sequence (for nftables)")
+    ops.add_argument('--list-ops', action='store_true',
+                     help="List available --do and --dump operations")
+    ops.add_argument('--list-msgs', action='store_true',
+                     help="List all messages of the family (incl. notifications)")
+    ops.add_argument('--list-attrs', dest='list_attrs', metavar='MSG',
+                     type=str, help='List attributes for a message / operation')
+    ops.add_argument('--validate', action='store_true',
+                     help="Validate the spec against schema and exit")
+
+    io_group = parser.add_argument_group('Input / Output')
+    io_group.add_argument('--json', dest='json_text', type=str,
+                          help=('Specify attributes of the message to send '
+                                'to the kernel in JSON format. Can be left out '
+                                'if the message is expected to be empty.'))
+    io_group.add_argument('--output-json', action='store_true',
+                          help='Format output as JSON')
+
+    ntf_group = parser.add_argument_group('Notifications')
+    ntf_group.add_argument('--subscribe', dest='ntf', type=str)
+    ntf_group.add_argument('--duration', dest='duration', type=int,
+                           help='when subscribed, watch for DURATION seconds')
+    ntf_group.add_argument('--sleep', dest='duration', type=int,
+                           help='alias for duration')
+
+    nlflags = parser.add_argument_group('Netlink message flags (NLM_F_*)',
+                                        ('Extra flags to set in nlmsg_flags of '
+                                         'the request, used mostly by older '
+                                         'Classic Netlink families.'))
+    nlflags.add_argument('--replace', dest='flags', action='append_const',
+                         const=Netlink.NLM_F_REPLACE)
+    nlflags.add_argument('--excl', dest='flags', action='append_const',
+                         const=Netlink.NLM_F_EXCL)
+    nlflags.add_argument('--create', dest='flags', action='append_const',
+                         const=Netlink.NLM_F_CREATE)
+    nlflags.add_argument('--append', dest='flags', action='append_const',
+                         const=Netlink.NLM_F_APPEND)
+
+    schema_group = parser.add_argument_group('Development options')
+    schema_group.add_argument('--schema', dest='schema', type=str,
+                              help="JSON schema to validate the spec")
+    schema_group.add_argument('--no-schema', action='store_true')
+
+    dbg_group = parser.add_argument_group('Debug options')
+    dbg_group.add_argument('--dbg-small-recv', default=0, const=4000,
+                           action='store', nargs='?', type=int, metavar='INT',
+                           help="Length of buffers used for recv()")
+    dbg_group.add_argument('--process-unknown', action=argparse.BooleanOptionalAction)
 
-    parser.add_argument('--duration', dest='duration', type=int,
-                        help='when subscribed, watch for DURATION seconds')
-    parser.add_argument('--sleep', dest='duration', type=int,
-                        help='alias for duration')
-    parser.add_argument('--subscribe', dest='ntf', type=str)
-    parser.add_argument('--replace', dest='flags', action='append_const',
-                        const=Netlink.NLM_F_REPLACE)
-    parser.add_argument('--excl', dest='flags', action='append_const',
-                        const=Netlink.NLM_F_EXCL)
-    parser.add_argument('--create', dest='flags', action='append_const',
-                        const=Netlink.NLM_F_CREATE)
-    parser.add_argument('--append', dest='flags', action='append_const',
-                        const=Netlink.NLM_F_APPEND)
-    parser.add_argument('--process-unknown', action=argparse.BooleanOptionalAction)
-    parser.add_argument('--output-json', action='store_true')
-    parser.add_argument('--dbg-small-recv', default=0, const=4000,
-                        action='store', nargs='?', type=int)
     args = parser.parse_args()
 
     def output(msg):

@@ -179,7 +179,7 @@ RELATIVE_SCHEMA_DIR='../../../../Documentation/netlink'
                      help="List available --do and --dump operations")
     ops.add_argument('--list-msgs', action='store_true',
                      help="List all messages of the family (incl. notifications)")
-    ops.add_argument('--list-attrs', dest='list_attrs', metavar='MSG',
+    ops.add_argument('--list-attrs', '--doc', dest='list_attrs', metavar='MSG',
                      type=str, help='List attributes for a message / operation')
     ops.add_argument('--validate', action='store_true',
                      help="Validate the spec against schema and exit")

@@ -137,6 +137,25 @@ RELATIVE_SCHEMA_DIR='../../../../Documentation/netlink'
         print_attr_list(ynl, mode_spec['attributes'], attr_set)
 
 
+def do_doc(ynl, op):
+    """Handle --list-attrs $op, print the attr information to stdout"""
+    print(f'Operation: {color(op.name, Colors.BOLD)}')
+    print(op.yaml['doc'])
+
+    for mode in ['do', 'dump', 'event']:
+        if mode in op.yaml:
+            print_mode_attrs(ynl, mode, op.yaml[mode], op.attr_set, True)
+
+    if 'notify' in op.yaml:
+        mode_spec = op.yaml['notify']
+        ref_spec = ynl.msgs.get(mode_spec).yaml.get('do')
+        if ref_spec:
+            print_mode_attrs(ynl, 'notify', ref_spec, op.attr_set, False)
+
+    if 'mcgrp' in op.yaml:
+        print(f"\nMulticast group: {op.yaml['mcgrp']}")
+
+
 # pylint: disable=too-many-locals,too-many-branches,too-many-statements
 def main():
     """YNL cli tool"""

@@ -286,21 +305,7 @@ RELATIVE_SCHEMA_DIR='../../../../Documentation/netlink'
             print(f'Operation {args.list_attrs} not found')
             sys.exit(1)
 
-        print(f'Operation: {color(op.name, Colors.BOLD)}')
-        print(op.yaml['doc'])
-
-        for mode in ['do', 'dump', 'event']:
-            if mode in op.yaml:
-                print_mode_attrs(ynl, mode, op.yaml[mode], op.attr_set, True)
-
-        if 'notify' in op.yaml:
-            mode_spec = op.yaml['notify']
-            ref_spec = ynl.msgs.get(mode_spec).yaml.get('do')
-            if ref_spec:
-                print_mode_attrs(ynl, 'notify', ref_spec, op.attr_set, False)
-
-        if 'mcgrp' in op.yaml:
-            print(f"\nMulticast group: {op.yaml['mcgrp']}")
+        do_doc(ynl, op)
 
     try:
         if args.do:

@@ -120,11 +120,11 @@ RELATIVE_SCHEMA_DIR='../../../../Documentation/netlink'
                     print_attr_list(ynl, nested_names, nested_set, indent + 4)
 
 
-def print_mode_attrs(ynl, mode, mode_spec, attr_set, print_request=True):
+def print_mode_attrs(ynl, mode, mode_spec, attr_set):
     """Print a given mode (do/dump/event/notify)."""
     mode_title = mode.capitalize()
 
-    if print_request and 'request' in mode_spec and 'attributes' in mode_spec['request']:
+    if 'request' in mode_spec and 'attributes' in mode_spec['request']:
         print(f'\n{mode_title} request attributes:')
         print_attr_list(ynl, mode_spec['request']['attributes'], attr_set)
 

@@ -132,25 +132,28 @@ RELATIVE_SCHEMA_DIR='../../../../Documentation/netlink'
         print(f'\n{mode_title} reply attributes:')
         print_attr_list(ynl, mode_spec['reply']['attributes'], attr_set)
 
-    if 'attributes' in mode_spec:
-        print(f'\n{mode_title} attributes:')
-        print_attr_list(ynl, mode_spec['attributes'], attr_set)
-
 
 def do_doc(ynl, op):
     """Handle --list-attrs $op, print the attr information to stdout"""
     print(f'Operation: {color(op.name, Colors.BOLD)}')
     print(op.yaml['doc'])
 
-    for mode in ['do', 'dump', 'event']:
+    for mode in ['do', 'dump']:
         if mode in op.yaml:
-            print_mode_attrs(ynl, mode, op.yaml[mode], op.attr_set, True)
+            print_mode_attrs(ynl, mode, op.yaml[mode], op.attr_set)
+
+    if 'attributes' in op.yaml.get('event', {}):
+        print('\nEvent attributes:')
+        print_attr_list(ynl, op.yaml['event']['attributes'], op.attr_set)
 
     if 'notify' in op.yaml:
         mode_spec = op.yaml['notify']
         ref_spec = ynl.msgs.get(mode_spec).yaml.get('do')
+        if not ref_spec:
+            ref_spec = ynl.msgs.get(mode_spec).yaml.get('dump')
         if ref_spec:
-            print_mode_attrs(ynl, 'notify', ref_spec, op.attr_set, False)
+            print('\nNotification attributes:')
+            print_attr_list(ynl, ref_spec['reply']['attributes'], op.attr_set)
 
     if 'mcgrp' in op.yaml:
         print(f"\nMulticast group: {op.yaml['mcgrp']}")

@@ -120,7 +120,7 @@ RELATIVE_SCHEMA_DIR='../../../../Documentation/netlink'
                     print_attr_list(ynl, nested_names, nested_set, indent + 4)
 
 
-def print_mode_attrs(ynl, mode, mode_spec, attr_set):
+def print_mode_attrs(ynl, mode, mode_spec, attr_set, consistent_dd_reply=None):
     """Print a given mode (do/dump/event/notify)."""
     mode_title = mode.capitalize()
 

@@ -129,8 +129,15 @@ RELATIVE_SCHEMA_DIR='../../../../Documentation/netlink'
         print_attr_list(ynl, mode_spec['request']['attributes'], attr_set)
 
     if 'reply' in mode_spec and 'attributes' in mode_spec['reply']:
-        print(f'\n{mode_title} reply attributes:')
-        print_attr_list(ynl, mode_spec['reply']['attributes'], attr_set)
+        if consistent_dd_reply and mode == "do":
+            title = None  # Dump handling will print in combined format
+        elif consistent_dd_reply and mode == "dump":
+            title = 'Do and Dump'
+        else:
+            title = f'{mode_title}'
+        if title:
+            print(f'\n{title} reply attributes:')
+            print_attr_list(ynl, mode_spec['reply']['attributes'], attr_set)
 
 
 def do_doc(ynl, op):

@@ -138,9 +145,15 @@ RELATIVE_SCHEMA_DIR='../../../../Documentation/netlink'
     print(f'Operation: {color(op.name, Colors.BOLD)}')
     print(op.yaml['doc'])
 
+    consistent_dd_reply = False
+    if 'do' in op.yaml and 'dump' in op.yaml and 'reply' in op.yaml['do'] and \
+       op.yaml['do']['reply'] == op.yaml['dump'].get('reply'):
+        consistent_dd_reply = True
+
     for mode in ['do', 'dump']:
         if mode in op.yaml:
-            print_mode_attrs(ynl, mode, op.yaml[mode], op.attr_set)
+            print_mode_attrs(ynl, mode, op.yaml[mode], op.attr_set,
+                             consistent_dd_reply=consistent_dd_reply)
 
     if 'attributes' in op.yaml.get('event', {}):
         print('\nEvent attributes:')