[PATCH]: Optional explanation for options

[Magnus Therning]

On Sat, Sep 10, 2005 at 02:09:17AM +1000, Robert Collins wrote:
[..]
>This is somewhat ugly: an api where type inspection is needed is prone
>to mistakes, and likely to have headaches. I think a better approach
>would be to done something along the lines of this - which is wrong,
>but demonstrates the concept:
>
>Make the entries in OPTIONS a list of tuples with default explanations.
>Then we get 
>class cmd_my_command(Command):
>    ...
>    takes_options = [OPTIONS[option] for option in ['revision', ...]]
>    ...
>
>For commands that want to override:
>class cmd_my_command(Command):
>    ...
>    takes_options = [(OPTIONS['revision'][0], "foo bar does abc")]
>    ...
>
>The things that are wrong is that this is overly verbose and unclear.

OK, here's a second stab at it. Let me know what you think. It isn't
complete, but it's enough to get my thought across, I hope.

Commands are written like this:

 class cmd_my_command(Command):
     ...
     takes_options = bzrlib.commands.prep_ops('option1', option2='description')
     ..

/M

*** modified file 'bzrlib/commands.py'
--- bzrlib/commands.py 
+++ bzrlib/commands.py 
@@ -358,34 +358,34 @@
 
 
 
-# list of all available options; the rhs can be either None for an
-# option that takes no argument, or a constructor function that checks
-# the type.
+# list of all available options and their descriptions; the rhs can be either
+# None for an option that takes no argument, or a constructor function that
+# checks the type.
 OPTIONS = {
-    'all':                    None,
-    'diff-options':           str,
-    'help':                   None,
-    'file':                   unicode,
-    'force':                  None,
-    'format':                 unicode,
-    'forward':                None,
-    'message':                unicode,
-    'no-recurse':             None,
-    'profile':                None,
-    'revision':               _parse_revision_str,
-    'short':                  None,
-    'show-ids':               None,
-    'timezone':               str,
-    'verbose':                None,
-    'version':                None,
-    'email':                  None,
-    'unchanged':              None,
-    'update':                 None,
-    'long':                   None,
-    'root':                   str,
-    'no-backup':              None,
-    'merge-type':             get_merge_type,
-    'pattern':                str,
+    'all':                    (None, 'description'),
+    'diff-options':           (str, 'description'),
+    'help':                   (None, 'description'),
+    'file':                   (unicode, 'description'),
+    'force':                  (None, 'description'),
+    'format':                 (unicode, 'description'),
+    'forward':                (None, 'description'),
+    'message':                (unicode, 'description'),
+    'no-recurse':             (None, 'description'),
+    'profile':                (None, 'description'),
+    'revision':               (_parse_revision_str, 'description'),
+    'short':                  (None, 'description'),
+    'show-ids':               (None, 'description'),
+    'timezone':               (str, 'description'),
+    'verbose':                (None, 'description'),
+    'version':                (None, 'description'),
+    'email':                  (None, 'description'),
+    'unchanged':              (None, 'description'),
+    'update':                 (None, 'description'),
+    'long':                   (None, 'description'),
+    'root':                   (str, 'description'),
+    'no-backup':              (None, 'description'),
+    'merge-type':             (get_merge_type, 'description'),
+    'pattern':                (str, 'description'),
     }
 
 SHORT_OPTIONS = {
@@ -397,6 +397,13 @@
     'l':                      'long',
 }
 
+def prep_ops(*args, **kwargs):
+    res = []
+    for arg in args:
+        res.append((arg, OPTIONS[arg][1]))
+    for kw in kwargs:
+        res.append((kw.replace('_', '-'), kwargs[kw]))
+    return res
 
 def parse_args(argv):
     """Parse command line.
@@ -467,7 +474,7 @@
                         # There are extra things on this option
                         # see if it is the value, or if it is another
                         # short option
-                        optargfn = OPTIONS[optname]
+                        optargfn = OPTIONS[optname][0]
                         if optargfn is None:
                             # This option does not take an argument, so the
                             # next entry is another short option, pack it back
@@ -482,7 +489,7 @@
                 # XXX: Do we ever want to support this, e.g. for -r?
                 raise BzrError('repeated option %r' % a)
                 
-            optargfn = OPTIONS[optname]
+            optargfn = OPTIONS[optname][0]
             if optargfn:
                 if optarg == None:
                     if not argv:

*** modified file 'bzrlib/help.py'
--- bzrlib/help.py 
+++ bzrlib/help.py 
@@ -127,11 +127,12 @@
 
     outfile.write('\noptions:\n')
     for on in options:
-        l = '    --' + on
+        l = '    --' + on[0]
         for shortname, longname in SHORT_OPTIONS.items():
-            if longname == on:
+            if longname == on[0]:
                 l += ', -' + shortname
                 break
+        l += (30 - len(l)) * ' ' + on[1]
         outfile.write(l + '\n')
 
 


-- 
Magnus Therning                    (OpenPGP: 0xAB4DFBA4)
magnus at therning.org
http://therning.org/magnus

Software is not manufactured, it is something you write and publish.
Keep Europe free from software patents, we do not want censorship
by patent law on written works.

Maturity is when you quit blaming other people for your problems
     -- Craig Burton
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20050910/eee73506/attachment.pgp