Rev 2605: cleanup global options, and require options to provide a help string in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Thu Jul 12 09:52:58 BST 2007
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 2605
revision-id: pqm at pqm.ubuntu.com-20070712085245-afvocysw990c1a3z
parent: pqm at pqm.ubuntu.com-20070712075207-pgz7ur4rxmklmrxr
parent: mbp at sourcefrog.net-20070711063329-3q9rz8bljj2iisi1
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Thu 2007-07-12 09:52:45 +0100
message:
cleanup global options, and require options to provide a help string
modified:
NEWS NEWS-20050323055033-4e00b5db738777ff
bzrlib/builtins.py builtins.py-20050830033751-fc01482b9ca23183
bzrlib/commands.py bzr.py-20050309040720-d10f4714595cf8c3
bzrlib/option.py option.py-20051014052914-661fb36e76e7362f
bzrlib/tests/test_options.py testoptions.py-20051014093702-96457cfc86319a8f
------------------------------------------------------------
revno: 2598.1.12
merged: mbp at sourcefrog.net-20070711063329-3q9rz8bljj2iisi1
parent: mbp at sourcefrog.net-20070711051842-rt9uj743zir4ixhn
committer: Martin Pool <mbp at sourcefrog.net>
branch nick: check-options
timestamp: Wed 2007-07-11 16:33:29 +1000
message:
Fix up --kind options
------------------------------------------------------------
revno: 2598.1.11
merged: mbp at sourcefrog.net-20070711051842-rt9uj743zir4ixhn
parent: mbp at sourcefrog.net-20070711050413-hrgdcuhrujn974gc
committer: Martin Pool <mbp at sourcefrog.net>
branch nick: check-options
timestamp: Wed 2007-07-11 15:18:42 +1000
message:
Insist that all options have a help string and fix those that don't.
(Thanks John)
------------------------------------------------------------
revno: 2598.1.10
merged: mbp at sourcefrog.net-20070711050413-hrgdcuhrujn974gc
parent: mbp at sourcefrog.net-20070711041852-u0owtlcd1rh9ri37
committer: Martin Pool <mbp at sourcefrog.net>
branch nick: check-options
timestamp: Wed 2007-07-11 15:04:13 +1000
message:
Clean up options that are registered globally and used once or not at all.
------------------------------------------------------------
revno: 2598.1.9
merged: mbp at sourcefrog.net-20070711041852-u0owtlcd1rh9ri37
parent: mbp at sourcefrog.net-20070711041548-n1om2ptyj2j01r6l
committer: Martin Pool <mbp at sourcefrog.net>
branch nick: check-options
timestamp: Wed 2007-07-11 14:18:52 +1000
message:
Add failing test that global options are used at least twice
------------------------------------------------------------
revno: 2598.1.8
merged: mbp at sourcefrog.net-20070711041548-n1om2ptyj2j01r6l
parent: mbp at sourcefrog.net-20070711034822-w0c7cnxnuo7naay5
committer: Martin Pool <mbp at sourcefrog.net>
branch nick: check-options
timestamp: Wed 2007-07-11 14:15:48 +1000
message:
Clean up old/unused global options
------------------------------------------------------------
revno: 2598.1.7
merged: mbp at sourcefrog.net-20070711034822-w0c7cnxnuo7naay5
parent: mbp at sourcefrog.net-20070711034634-44v65lrdqsdh9k2s
committer: Martin Pool <mbp at sourcefrog.net>
branch nick: check-options
timestamp: Wed 2007-07-11 13:48:22 +1000
message:
Add failing test that global options are all used.
------------------------------------------------------------
revno: 2598.1.6
merged: mbp at sourcefrog.net-20070711034634-44v65lrdqsdh9k2s
parent: mbp at sourcefrog.net-20070711032003-9t03vnewhh4fg6qw
committer: Martin Pool <mbp at sourcefrog.net>
branch nick: check-options
timestamp: Wed 2007-07-11 13:46:34 +1000
message:
Add help for --no-backup
=== modified file 'NEWS'
--- a/NEWS 2007-07-12 03:55:39 +0000
+++ b/NEWS 2007-07-12 08:52:45 +0000
@@ -13,7 +13,11 @@
LIBRARY API BREAKS:
- * None yet ...
+ * Deprecated dictionary ``bzrlib.option.SHORT_OPTIONS`` removed.
+ Various globally-declared options removed. Options are now required
+ to provide a help string and it must comply with the style guide
+ by being one or more sentences with an initial capital and final
+ period. (Martin Pool)
INTERNALS:
=== modified file 'bzrlib/builtins.py'
--- a/bzrlib/builtins.py 2007-07-12 03:55:39 +0000
+++ b/bzrlib/builtins.py 2007-07-12 08:52:45 +0000
@@ -338,9 +338,16 @@
into a subdirectory of this one.
"""
takes_args = ['file*']
- takes_options = ['no-recurse', 'dry-run', 'verbose',
- Option('file-ids-from', type=unicode,
- help='Lookup file ids from this tree.')]
+ takes_options = [
+ Option('no-recurse',
+ help="Don't recursively add the contents of directories."),
+ Option('dry-run',
+ help="Show what would be done, but don't actually do anything."),
+ 'verbose',
+ Option('file-ids-from',
+ type=unicode,
+ help='Lookup file ids from this tree.'),
+ ]
encoding_type = 'replace'
_see_also = ['remove']
@@ -436,13 +443,19 @@
hidden = True
_see_also = ['ls']
- takes_options = ['revision', 'show-ids', 'kind']
+ takes_options = [
+ 'revision',
+ 'show-ids',
+ Option('kind',
+ help='List entries of a particular kind: file, directory, symlink.',
+ type=unicode),
+ ]
takes_args = ['file*']
@display_command
def run(self, revision=None, show_ids=False, kind=None, file_list=None):
if kind and kind not in ['file', 'directory', 'symlink']:
- raise errors.BzrCommandError('invalid kind specified')
+ raise errors.BzrCommandError('invalid kind %r specified' % (kind,))
work_tree, file_list = tree_files(file_list)
work_tree.lock_read()
@@ -1386,11 +1399,14 @@
_see_also = ['status']
takes_args = ['file*']
- takes_options = ['revision', 'diff-options',
+ takes_options = [
+ Option('diff-options', type=str,
+ help='Pass these options to the external diff program.'),
Option('prefix', type=str,
short_name='p',
help='Set prefixes to added to old and new filenames, as '
'two values separated by a colon. (eg "old/:new/").'),
+ 'revision',
]
aliases = ['di', 'dif']
encoding_type = 'exact'
@@ -1584,7 +1600,9 @@
takes_options = [
Option('forward',
help='Show from oldest to newest.'),
- 'timezone',
+ Option('timezone',
+ type=str,
+ help='Display timezone as local, original, or utc.'),
Option('verbose',
short_name='v',
help='Show files changed in each revision.'),
@@ -1732,11 +1750,13 @@
Option('null',
help='Write an ascii NUL (\\0) separator '
'between files rather than a newline.'),
- 'kind',
+ Option('kind',
+ help='List entries of a particular kind: file, directory, symlink.',
+ type=unicode),
'show-ids',
]
@display_command
- def run(self, revision=None, verbose=False,
+ def run(self, revision=None, verbose=False,
non_recursive=False, from_root=False,
unknown=False, versioned=False, ignored=False,
null=False, kind=None, show_ids=False, path=None):
@@ -1975,7 +1995,15 @@
zip .zip
"""
takes_args = ['dest', 'branch?']
- takes_options = ['revision', 'format', 'root']
+ takes_options = [
+ Option('format',
+ help="Type of file to export to.",
+ type=unicode),
+ 'revision',
+ Option('root',
+ type=str,
+ help="Name of the root directory inside the exported file."),
+ ]
def run(self, dest, branch=None, revision=None, format=None, root=None):
from bzrlib.export import export
@@ -2009,7 +2037,10 @@
"""
_see_also = ['ls']
- takes_options = ['revision', 'name-from-revision']
+ takes_options = [
+ Option('name-from-revision', help='The path name in the old tree.'),
+ 'revision',
+ ]
takes_args = ['filename']
encoding_type = 'exact'
@@ -2103,7 +2134,9 @@
_see_also = ['bugs', 'uncommit']
takes_args = ['selected*']
takes_options = [
- 'message',
+ Option('message', type=unicode,
+ short_name='m',
+ help="Description of the new revision."),
'verbose',
Option('unchanged',
help='Commit even if nothing has changed.'),
@@ -2571,7 +2604,13 @@
_see_also = ['update', 'remerge', 'status-flags']
takes_args = ['branch?']
- takes_options = ['revision', 'force', 'merge-type', 'reprocess', 'remember',
+ takes_options = [
+ 'revision',
+ Option('force',
+ help='Merge even if the destination tree has uncommitted changes.'),
+ 'merge-type',
+ 'reprocess',
+ 'remember',
Option('show-base', help="Show base revision text in "
"conflicts."),
Option('uncommitted', help='Apply uncommitted changes'
@@ -2581,11 +2620,11 @@
' source rather than merging. When this happens,'
' you do not need to commit the result.'),
Option('directory',
- help='Branch to merge into, '
- 'rather than the one containing the working directory.',
- short_name='d',
- type=unicode,
- ),
+ help='Branch to merge into, '
+ 'rather than the one containing the working directory.',
+ short_name='d',
+ type=unicode,
+ ),
]
def run(self, branch=None, revision=None, force=False, merge_type=None,
@@ -2840,7 +2879,10 @@
"""
_see_also = ['cat', 'export']
- takes_options = ['revision', 'no-backup']
+ takes_options = [
+ 'revision',
+ Option('no-backup', "Do not save backups of reverted files."),
+ ]
takes_args = ['file*']
def run(self, revision=None, no_backup=False, file_list=None):
=== modified file 'bzrlib/commands.py'
--- a/bzrlib/commands.py 2007-07-04 01:39:50 +0000
+++ b/bzrlib/commands.py 2007-07-11 04:15:48 +0000
@@ -332,7 +332,7 @@
Maps from long option name to option object."""
r = dict()
- r['help'] = option.Option.OPTIONS['help']
+ r['help'] = option._help_option
for o in self.takes_options:
if isinstance(o, basestring):
o = option.Option.OPTIONS[o]
=== modified file 'bzrlib/option.py'
--- a/bzrlib/option.py 2007-07-11 02:32:38 +0000
+++ b/bzrlib/option.py 2007-07-11 05:18:42 +0000
@@ -152,13 +152,6 @@
def short_name(self):
if self._short_name:
return self._short_name
- else:
- # remove this when SHORT_OPTIONS is removed
- # XXX: This is accessing a DeprecatedDict, so we call the super
- # method to avoid warnings
- for (k, v) in dict.iteritems(Option.SHORT_OPTIONS):
- if v == self:
- return k
def set_short_name(self, short_name):
self._short_name = short_name
@@ -374,78 +367,27 @@
_merge_type_registry.register_lazy('weave', 'bzrlib.merge', 'WeaveMerger',
"Weave-based merge")
-_global_option('all')
_global_option('overwrite', help='Ignore differences between branches and '
'overwrite unconditionally.')
-_global_option('basis', type=str)
-_global_option('bound')
-_global_option('diff-options', type=str)
-_global_option('help',
- 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,
- short_name='m')
-_global_option('no-recurse')
-_global_option('profile',
- help='Show performance profiling information.')
_global_option('revision',
type=_parse_revision_str,
short_name='r',
help='See \'help revisionspec\' for details.')
_global_option('show-ids',
help='Show internal object ids.')
-_global_option('timezone',
- type=str,
- help='Display timezone as local, original, or utc.')
-_global_option('unbound')
_global_option('verbose',
help='Display more information.',
short_name='v')
-_global_option('version')
-_global_option('email')
-_global_option('update')
_global_registry_option('log-format', "Use specified log format.",
log.log_formatter_registry, value_switches=True,
title='Log format')
-_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)
-_global_option('no-backup')
_global_registry_option('merge-type', 'Select a particular merge algorithm.',
_merge_type_registry, value_switches=True,
title='Merge algorithm')
-_global_option('pattern', type=str)
-_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.')
-_global_option('kind', type=str)
-_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.')
-
-
-# prior to 0.14 these were always globally registered; the old dict is
-# available for plugins that use it but it should not be used.
-Option.SHORT_OPTIONS = symbol_versioning.DeprecatedDict(
- symbol_versioning.zero_fourteen,
- 'SHORT_OPTIONS',
- {
- 'F': Option.OPTIONS['file'],
- 'h': Option.OPTIONS['help'],
- 'm': Option.OPTIONS['message'],
- 'r': Option.OPTIONS['revision'],
- 'v': Option.OPTIONS['verbose'],
- 'l': Option.OPTIONS['long'],
- 'q': Option.OPTIONS['quiet'],
- },
- 'Set the short option name when constructing the Option.',
- )
+
+_help_option = Option('help',
+ help='Show help message.',
+ short_name='h')
=== modified file 'bzrlib/tests/test_options.py'
--- a/bzrlib/tests/test_options.py 2007-07-11 02:32:38 +0000
+++ b/bzrlib/tests/test_options.py 2007-07-11 05:18:42 +0000
@@ -74,45 +74,11 @@
out, err = self.run_bzr('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_set_short_name(self):
o = option.Option('wiggle')
o.set_short_name('w')
self.assertEqual(o.short_name(), 'w')
- def test_old_short_names(self):
- # test the deprecated method for getting and setting short option
- # names
- expected_warning = (
- "access to SHORT_OPTIONS was deprecated in version 0.14."
- " Set the short option name when constructing the Option.",
- DeprecationWarning, 2)
- _warnings = []
- def capture_warning(message, category, stacklevel=None):
- _warnings.append((message, category, stacklevel))
- old_warning_method = symbol_versioning.warn
- try:
- # an example of the kind of thing plugins might want to do through
- # the old interface - make a new option and then give it a short
- # name.
- symbol_versioning.set_warning_method(capture_warning)
- example_opt = option.Option('example', help='example option')
- option.Option.SHORT_OPTIONS['w'] = example_opt
- self.assertEqual(example_opt.short_name(), 'w')
- self.assertEqual([expected_warning], _warnings)
- # now check that it can actually be parsed with the registered
- # value
- opts, args = parse([example_opt], ['-w', 'foo'])
- self.assertEqual(opts.example, True)
- self.assertEqual(args, ['foo'])
- finally:
- symbol_versioning.set_warning_method(old_warning_method)
-
def test_allow_dash(self):
"""Test that we can pass a plain '-' as an argument."""
self.assertEqual(
@@ -296,6 +262,35 @@
self.assertTrue(len(all) > 100,
"too few options found: %r" % all)
+ def test_global_options_used(self):
+ # In the distant memory, options could only be declared globally. Now
+ # we prefer to declare them in the command, unless like -r they really
+ # are used very widely with the exact same meaning. So this checks
+ # for any that should be garbage collected.
+ g = dict(option.Option.OPTIONS.items())
+ used_globals = {}
+ msgs = []
+ for cmd_name, cmd_class in sorted(commands.get_all_cmds()):
+ for option_or_name in sorted(cmd_class.takes_options):
+ if not isinstance(option_or_name, basestring):
+ self.assertIsInstance(option_or_name, option.Option)
+ elif not option_or_name in g:
+ msgs.append("apparent reference to undefined "
+ "global option %r from %r"
+ % (option_or_name, cmd_class))
+ else:
+ used_globals.setdefault(option_or_name, []).append(cmd_name)
+ unused_globals = set(g.keys()) - set(used_globals.keys())
+ for option_name in sorted(unused_globals):
+ msgs.append("unused global option %r" % option_name)
+ for option_name, cmds in sorted(used_globals.items()):
+ if len(cmds) <= 1:
+ msgs.append("global option %r is only used by %r"
+ % (option_name, cmds))
+ if msgs:
+ self.fail("problems with global option definitions:\n"
+ + '\n'.join(msgs))
+
def test_option_grammar(self):
msgs = []
# Option help should be written in sentence form, and have a final
@@ -304,9 +299,9 @@
option_re = re.compile(r'^[A-Z][^\n]+\.$')
for scope, option in self.get_all_options():
if not option.help:
- # TODO: Also complain about options that have no help message?
- continue
- if not option_re.match(option.help):
+ msgs.append('%-16s %-16s %s' %
+ ((scope or 'GLOBAL'), option.name, 'NO HELP'))
+ elif not option_re.match(option.help):
msgs.append('%-16s %-16s %s' %
((scope or 'GLOBAL'), option.name, option.help))
if msgs:
More information about the bazaar-commits
mailing list