On Mon, 2012-01-30 at 11:22 -0500, Rob Crittenden wrote:
> Martin Kosek wrote:
> > An example of the improved help:
> >
> > # ipa help user-add
> > Purpose: Add a new user.
> > Usage: ipa [global-options] user-add LOGIN [options]
> >
> > Positional arguments:
> > LOGIN: User login
> >
> > Options:
> >    -h, --help         show this help message and exit
> >    --first=STR        First name
> >    --last=STR         Last name
> >    --cn=STR           Full name
> > ...
> >
> >
> > We may want to improve help for most arguments we use. Most of them are
> > missing. In this patch I updated just the CRITERIA argument that was
> > complained about in the relevant BZ.
> >
> > Martin
> 
> Patch looks ok, I just think the output could be improved.
> 
> I think it should look similar to the existing usage output, so, similar 
> indention and description columns lined up:
> 
> Usage: ipa [global-options] user-add LOGIN [options]
> 
> Positional arguments:
>    LOGIN              User login
> 
> Options:
>    -h, --help         show this help message and exit
> 
> rob

I agree. I refactored the patch to integrate better with OptionParser
and rather provide a support of argument help directly instead of
misusing description field.

New patch adds a formatter capable of formatting arguments consistently
with options format (as you proposed).

Martin
>From 2af51484aa097c944445b6b3639fd47c103d477f Mon Sep 17 00:00:00 2001
From: Martin Kosek <[email protected]>
Date: Fri, 27 Jan 2012 16:51:37 +0100
Subject: [PATCH] Add argument help to CLI

CLI command help contains a documentation for all options that can
be passed to commands. However, help strings for positional
arguments are not included.

This patch uses an OptionParser description field to list all
command arguments as OptionParser does not have a native support
to provide such information to user.

https://fedorahosted.org/freeipa/ticket/1974
---
 ipalib/cli.py              |   86 +++++++++++++++++++++++++++++++++++++++-----
 ipalib/plugins/baseldap.py |    4 ++-
 2 files changed, 80 insertions(+), 10 deletions(-)

diff --git a/ipalib/cli.py b/ipalib/cli.py
index 667a7cdc40b4557841bac3186b3271836e6f358c..c5bb9461fddc8ec20190778d0f51d1629e8659ec 100644
--- a/ipalib/cli.py
+++ b/ipalib/cli.py
@@ -942,6 +942,57 @@ class Collector(object):
     def __todict__(self):
         return dict(self.__options)
 
+class CLIOptionParserFormatter(optparse.IndentedHelpFormatter):
+    def format_argument(self, name, help_string):
+        result = []
+        opt_width = self.help_position - self.current_indent - 2
+        if len(name) > opt_width:
+            name = "%*s%s\n" % (self.current_indent, "", name)
+            indent_first = self.help_position
+        else:                       # start help on same line as name
+            name = "%*s%-*s  " % (self.current_indent, "", opt_width, name)
+            indent_first = 0
+        result.append(name)
+        if help_string:
+            help_lines = textwrap.wrap(help_string, self.help_width)
+            result.append("%*s%s\n" % (indent_first, "", help_lines[0]))
+            result.extend(["%*s%s\n" % (self.help_position, "", line)
+                           for line in help_lines[1:]])
+        elif name[-1] != "\n":
+            result.append("\n")
+        return "".join(result)
+
+class CLIOptionParser(optparse.OptionParser):
+    """
+    This OptionParser subclass adds an ability to print positional
+    arguments in CLI help. Custom formatter is used to format the argument
+    list in the same way as OptionParser formats options.
+    """
+    def __init__(self, *args, **kwargs):
+        self._arguments = []
+        if 'formatter' not in kwargs:
+            kwargs['formatter'] = CLIOptionParserFormatter()
+        optparse.OptionParser.__init__(self, *args, **kwargs)
+
+    def format_option_help(self, formatter=None):
+        """
+        Prepend argument help to standard OptionParser's option help
+        """
+        option_help = optparse.OptionParser.format_option_help(self, formatter)
+
+        if isinstance(formatter, CLIOptionParserFormatter):
+            arguments = []
+            for (name, help_string) in self._arguments:
+                arguments.append(formatter.format_argument(name, help_string))
+            if arguments:
+                heading = unicode(_("Positional arguments"))
+                arguments.insert(0, formatter.format_heading(heading))
+                arguments.append("\n")
+            option_help = "".join(arguments) + option_help
+        return option_help
+
+    def add_argument(self, name, help_string):
+        self._arguments.append((name, help_string))
 
 class cli(backend.Executioner):
     """
@@ -1006,7 +1057,7 @@ class cli(backend.Executioner):
                 yield (key, self.Backend.textui.decode(value))
 
     def build_parser(self, cmd):
-        parser = optparse.OptionParser(
+        parser = CLIOptionParser(
             usage=' '.join(self.usage_iter(cmd))
         )
         option_groups = {}
@@ -1045,20 +1096,37 @@ class cli(backend.Executioner):
                 option_group.add_option(o)
             else:
                 parser.add_option(o)
+
+        for arg in cmd.args():
+            name = self.__get_arg_name(arg, format_name=False)
+            if name is None:
+                continue
+            doc = unicode(arg.doc)
+            parser.add_argument(name, doc)
+
         return parser
 
+    def __get_arg_name(self, arg, format_name=True):
+        if arg.password:
+            return
+
+        name = to_cli(arg.cli_name).upper()
+        if not format_name:
+            return name
+        if arg.multivalue:
+            name = '%s...' % name
+        if arg.required:
+            return name
+        else:
+            return '[%s]' % name
+
     def usage_iter(self, cmd):
         yield 'Usage: %%prog [global-options] %s' % to_cli(cmd.name)
         for arg in cmd.args():
-            if arg.password:
+            name = self.__get_arg_name(arg)
+            if name is None:
                 continue
-            name = to_cli(arg.cli_name).upper()
-            if arg.multivalue:
-                name = '%s...' % name
-            if arg.required:
-                yield name
-            else:
-                yield '[%s]' % name
+            yield name
         yield '[options]'
 
     def prompt_interactively(self, cmd, kw):
diff --git a/ipalib/plugins/baseldap.py b/ipalib/plugins/baseldap.py
index f59a0d4106729573cfdbd8bdf2c407619eb051d5..00ae94939ffbea97fd54016abc8a0f3709f883f8 100644
--- a/ipalib/plugins/baseldap.py
+++ b/ipalib/plugins/baseldap.py
@@ -1591,7 +1591,9 @@ class LDAPSearch(BaseLDAPCommand, crud.Search):
         #pylint: disable=E1003
         for key in self.obj.get_ancestor_primary_keys():
             yield key
-        yield Str('criteria?', noextrawhitespace=False)
+        yield Str('criteria?',
+                  noextrawhitespace=False,
+                  doc=_('A string searched in all relevant object attributes'))
         for arg in super(crud.Search, self).get_args():
             yield arg
 
-- 
1.7.7.6

_______________________________________________
Freeipa-devel mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to