[patch] remove global short-option table

Martin Pool mbp at canonical.com
Tue Dec 19 06:25:09 GMT 2006


This patch removes the global registry of short option names.  If an
option has a single-letter name, that can now just be specified in the
Option constructor.  This removes some cruft.

Of course it's still a good idea to keep options consistent between
commands when possible, though the help text may usefully differ.

This changes Option.short_name to be a field not a method, which seems
unlikely to affect plugin code.

=== modified file 'NEWS'
--- NEWS	2006-12-19 05:40:29 +0000
+++ NEWS	2006-12-19 05:47:31 +0000
@@ -66,6 +66,9 @@
 
     * Test suite ends cleanly on Windows.  (Vincent Ladeuil)
 
+    * Single-letter short options are no longer globally declared.  (Martin
+      Pool)
+
   BUG FIXES:
 
     * ``bzr annotate`` now defaults to showing dotted revnos for merged

=== modified file 'bzrlib/builtins.py'
--- bzrlib/builtins.py	2006-12-15 02:14:12 +0000
+++ bzrlib/builtins.py	2006-12-19 06:09:09 +0000
@@ -1215,7 +1215,12 @@
     # TODO: This probably handles non-Unix newlines poorly.
     
     takes_args = ['file*']
-    takes_options = ['revision', 'diff-options', 'prefix']
+    takes_options = ['revision', 'diff-options',
+        Option('prefix', type=str,
+               short_name='p',
+               help='Set prefixes to added to old and new filenames, as '
+                    'two values separated by a colon.'),
+        ]
     aliases = ['di', 'dif']
     encoding_type = 'exact'
 
@@ -1234,7 +1239,7 @@
         else:
             if not ':' in prefix:
                  raise BzrCommandError(
-                     "--diff-prefix expects two values separated by a colon")
+                     "--prefix expects two values separated by a colon")
             old_label, new_label = prefix.split(":")
         
         try:
@@ -1375,11 +1380,13 @@
                             help='show from oldest to newest'),
                      'timezone', 
                      Option('verbose', 
+                             short_name='v',
                              help='show files changed in each revision'),
                      'show-ids', 'revision',
                      'log-format',
                      'line', 'long', 
                      Option('message',
+                            short_name='m',
                             help='show revisions whose message matches this regexp',
                             type=str),
                      'short',
@@ -1829,6 +1836,7 @@
                      Option('unchanged',
                             help='commit even if nothing has changed'),
                      Option('file', type=str, 
+                            short_name='F',
                             argname='msgfile',
                             help='file containing commit message'),
                      Option('strict',

=== modified file 'bzrlib/option.py'
--- bzrlib/option.py	2006-11-02 05:50:57 +0000
+++ bzrlib/option.py	2006-12-18 08:30:35 +0000
@@ -110,13 +110,18 @@
         raise errors.BzrCommandError(msg)
 
 class Option(object):
-    """Description of a command line option"""
+    """Description of a command line option
+    
+    :ivar short_name: If this option has a single-letter name, this is it.
+    Otherwise None.
+    """
+
     # TODO: Some way to show in help a description of the option argument
 
     OPTIONS = {}
-    SHORT_OPTIONS = {}
 
-    def __init__(self, name, help='', type=None, argname=None):
+    def __init__(self, name, help='', type=None, argname=None,
+                 short_name=None):
         """Make a new command option.
 
         name -- regular name of the command, used in the double-dash
@@ -130,26 +135,16 @@
 
         argname -- name of option argument, if any
         """
-        # TODO: perhaps a subclass that automatically does 
-        # --option, --no-option for reversible booleans
         self.name = name
         self.help = help
         self.type = type
+        self.short_name = short_name
         if type is None:
             assert argname is None
         elif argname is None:
             argname = 'ARG'
         self.argname = argname
 
-    def short_name(self):
-        """Return the single character option for this command, if any.
-
-        Short options are globally registered.
-        """
-        for short, option in Option.SHORT_OPTIONS.iteritems():
-            if option is self:
-                return short
-
     def get_negation_name(self):
         if self.name.startswith('no-'):
             return self.name[3:]
@@ -189,7 +184,7 @@
         argname =  self.argname
         if argname is not None:
             argname = argname.upper()
-        yield self.name, self.short_name(), argname, self.help
+        yield self.name, self.short_name, argname, self.help
 
 
 class OptionParser(optparse.OptionParser):
@@ -206,10 +201,8 @@
 
     parser = OptionParser()
     parser.remove_option('--help')
-    short_options = dict((k.name, v) for v, k in 
-                         Option.SHORT_OPTIONS.iteritems())
     for option in options.itervalues():
-        option.add_option(parser, short_options.get(option.name))
+        option.add_option(parser, option.short_name)
     return parser
 
 
@@ -224,19 +217,20 @@
 _global_option('bound')
 _global_option('diff-options', type=str)
 _global_option('help',
-               help='show help message')
-_global_option('file', type=unicode)
+               help='show help message',
+               short_name='h')
+_global_option('file', type=unicode, short_name='F')
 _global_option('force')
 _global_option('format', type=unicode)
 _global_option('forward')
-_global_option('message', type=unicode)
+_global_option('message', type=unicode,
+               short_name='m')
 _global_option('no-recurse')
-_global_option('prefix', type=str, 
-               help='Set prefixes to added to old and new filenames, as '
-                    'two values separated by a colon.')
 _global_option('profile',
                help='show performance profiling information')
-_global_option('revision', type=_parse_revision_str)
+_global_option('revision', 
+               type=_parse_revision_str,
+               short_name='r')
 _global_option('show-ids', 
                help='show internal object ids')
 _global_option('timezone', 
@@ -244,12 +238,14 @@
                help='display timezone as local, original, or utc')
 _global_option('unbound')
 _global_option('verbose',
-               help='display more information')
+               help='display more information',
+               short_name='v')
 _global_option('version')
 _global_option('email')
 _global_option('update')
 _global_option('log-format', type=str, help="Use this log format")
-_global_option('long', help='Use detailed log format. Same as --log-format long')
+_global_option('long', help='Use detailed log format. Same as --log-format long',
+               short_name='l')
 _global_option('short', help='Use moderately short log format. Same as --log-format short')
 _global_option('line', help='Use log format with one line per revision. Same as --log-format line')
 _global_option('root', type=str)
@@ -257,7 +253,7 @@
 _global_option('merge-type', type=_parse_merge_type, 
                help='Select a particular merge algorithm')
 _global_option('pattern', type=str)
-_global_option('quiet')
+_global_option('quiet', short_name='q')
 _global_option('remember', help='Remember the specified location as a'
                ' default.')
 _global_option('reprocess', help='Reprocess to reduce spurious conflicts')
@@ -265,18 +261,3 @@
 _global_option('dry-run',
                help="show what would be done, but don't actually do anything")
 _global_option('name-from-revision', help='The path name in the old tree.')
-
-
-def _global_short(short_name, long_name):
-    assert short_name not in Option.SHORT_OPTIONS
-    Option.SHORT_OPTIONS[short_name] = Option.OPTIONS[long_name]
-    
-
-Option.SHORT_OPTIONS['F'] = Option.OPTIONS['file']
-Option.SHORT_OPTIONS['h'] = Option.OPTIONS['help']
-Option.SHORT_OPTIONS['m'] = Option.OPTIONS['message']
-Option.SHORT_OPTIONS['r'] = Option.OPTIONS['revision']
-Option.SHORT_OPTIONS['v'] = Option.OPTIONS['verbose']
-Option.SHORT_OPTIONS['l'] = Option.OPTIONS['long']
-Option.SHORT_OPTIONS['q'] = Option.OPTIONS['quiet']
-Option.SHORT_OPTIONS['p'] = Option.OPTIONS['prefix']

=== modified file 'bzrlib/shellcomplete.py'
--- bzrlib/shellcomplete.py	2006-11-08 02:30:17 +0000
+++ bzrlib/shellcomplete.py	2006-12-18 08:30:36 +0000
@@ -25,7 +25,8 @@
     else:
         shellcomplete_on_command(context, outfile = outfile)
 
-def shellcomplete_on_command(cmdname, outfile = None):
+
+def shellcomplete_on_command(cmdname, outfile=None):
     cmdname = str(cmdname)
 
     if outfile is None:
@@ -39,25 +40,18 @@
     if doc is None:
         raise NotImplementedError("sorry, no detailed shellcomplete yet for %r" % cmdname)
 
-    shellcomplete_on_option(cmdobj.takes_options, outfile = None)
+    shellcomplete_on_options(cmdobj.options().values(), outfile=outfile)
     for aname in cmdobj.takes_args:
         outfile.write(aname + '\n')
 
 
-def shellcomplete_on_option(options, outfile=None):
-    from bzrlib.option import Option
-    if not options:
-        return
-    if outfile is None:
-        outfile = sys.stdout
-    for on in options:
-        for shortname, longname in Option.SHORT_OPTIONS.items():
-            if longname == on:
-                l = '"(--' + on + ' -' + shortname + ')"{--' + on + ',-' + shortname + '}'
-                break
-            else:
-                l = '--' + on
-        outfile.write(l + '\n')
+def shellcomplete_on_options(options, outfile=None):
+    for opt in options:
+        if opt.short_name:
+            outfile.write('"(--%s -%s)"{--%s,-%s}\n'
+                    % (opt.name, opt.short_name, opt.name, opt.short_name))
+        else:
+            outfile.write('--%s\n' % opt.name)
 
 
 def shellcomplete_commands(outfile = None):

=== modified file 'bzrlib/tests/test_options.py'
--- bzrlib/tests/test_options.py	2006-10-18 05:15:49 +0000
+++ bzrlib/tests/test_options.py	2006-12-18 08:30:35 +0000
@@ -63,6 +63,12 @@
         out, err = self.run_bzr_captured(['help', '-r'], retcode=3)
         self.assertContainsRe(err, r'no such option')
 
+    def test_get_short_name(self):
+        file_opt = option.Option.OPTIONS['file']
+        self.assertEquals(file_opt.short_name, 'F')
+        force_opt = option.Option.OPTIONS['force']
+        self.assertEquals(force_opt.short_name, None)
+
     def test_allow_dash(self):
         """Test that we can pass a plain '-' as an argument."""
         self.assertEqual((['-'], {}), parse_args(cmd_commit(), ['-']))

-- 
Martin




More information about the bazaar mailing list