[MERGE] command defaults

[John A Meinel]

Aaron Bentley wrote:
> Hi all,
> I've implemented per-user command defaults, in what I hope is a fairly
> tasteful way.
> 
> In ~/.bazaar/bazzar.conf, you can now have a COMMAND_DEFAULTS section.
> If you do, the 'name' is the command name, and the 'value' is a set of
> options and arguments.  As in all config files, # can be used to supply
> a comment.
> 
> Example:
> [COMMAND_DEFAULTS]
> commit = --strict # doncha hate it when you forget to add a file?
> revert = --no-backup
> log = --short -r -1..-5
> merge = --reprocess
> 
> When an option is supplied on the commandline that is also supplied in
> the defaults, the default for that value is ignored.  For example:
> $bzr log -r -1..-10
> would combine with the above defaults to produce, effectively:
> $ bzr log --short -r -1..-10
> 
> Additionally, a --no-defaults global option is available to prevent any
> defaults from being used.  (Especially for scripts and the test suite.)
> 
> Branch:
> http://panoramicfeedback.com/opensource/bzr/bzr.ab/
> 
> Patch attached.
> 
> Areas for future improvement:
> 1. the test suite should supply a default home directory
> 2. argument parsing should support enumerations: that is,
> log --short, --long and --line are all really different values for the
> same option.  In the ideal, we'd have something like
> OptionEnum('log-format', 'short', 'long', 'line').  The user could
> specify 'log --line', this would be parsed to {'log-format', 'line'},
> and it would override a default of log=--short
> 
> The same approach would apply to merge-type.
> 
> Aaron

Very nice. +1 from me. A couple small code issues:


...

=== modified file 'bzrlib/errors.py'
--- bzrlib/errors.py	
+++ bzrlib/errors.py	
@@ -539,3 +539,6 @@
     def __init__(self, format):
         BzrNewError.__init__(self)
         self.format = format
+
+class CommandDefaultSyntax(BzrNewError):
+    """Sytax error in defaults for "%(command_name)s": %(error)s"""


This should have 2 blank lines, and usually I would add:

  def __init__(self, command_name, error):
    BzrNewError.__init__(self, command_name=command_name, error=error)

That way you are sure at the time you raise the error that you have the
correct parameters, rather than when it is displayed as a string later.

(The test suite would catch the former, because it would get an
incorrect variable exception, while the latter wouldn't be found,
because it never asks for str(e))



=== modified file 'bzrlib/tests/__init__.py'
--- bzrlib/tests/__init__.py	
+++ bzrlib/tests/__init__.py	
@@ -438,6 +438,10 @@
         handler.setLevel(logging.INFO)
         logger = logging.getLogger('')
         logger.addHandler(handler)
+        if isinstance(argv, basestring):
+            argv = '--no-defaults ' + argv
+        else:
+            argv = ['--no-defaults'] + list(argv)
         try:
             result = self.apply_redirected(None, stdout, stderr,

bzrlib.commands.run_bzr_catch_errors,

=== modified file 'bzrlib/tests/test_config.py'
--- bzrlib/tests/test_config.py	
+++ bzrlib/tests/test_config.py	
@@ -32,7 +32,10 @@
                       "email=Robert Collins <robertc at example.com>\n"
                       "editor=vim\n"
                       "gpg_signing_command=gnome-gpg\n"
-                      "user_global_option=something\n")
+                      "user_global_option=something\n"
+                      "[COMMAND_DEFAULTS]\n"
+                      'commit=-m "log message" #evil? possibly.\n'
+                      'merge=--merge-type "weave')


 sample_always_signatures = ("[DEFAULT]\n"
@@ -352,6 +355,13 @@
         my_config = self._get_sample_config()
         self.assertEqual(None, my_config.post_commit())

+    def test_command_defaults(self):
+        my_config = self._get_sample_config()
+        self.assertEqual(['-m', 'log message'],
+                         my_config.get_command_defaults('commit'))
+        self.assertEqual([], my_config.get_command_defaults('log'))
+        self.assertRaises(errors.CommandDefaultSyntax,
+                          my_config.get_command_defaults, 'merge')

 class TestLocationConfig(TestCase):


This also needs a second line between the function and the next class.

Otherwise, it all looks good.

John
=:->


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 249 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060202/c9063fde/attachment.pgp