[bug 50984] handle "-" as an argument

[Martin Pool]

On 23 Jun 2006, Robert Widhopf-Fenk <hack at robf.de> wrote:
Content-Description: message body text
> A typo and its result.
> 
> I am not sure if the attached patch is the right fix ...

Thanks Robert.

  https://launchpad.net/products/bzr/+bug/50984

The fix looks good to me, but we should add a test.  That code is a bit
complex, so we want to make sure that the bug will not revert when
someone refactors it (or when someone changes something else in the
complex code.)  So here is a patch with a test.

Some people have asked how we get tests for this.  It's true that adding
a test takes more time than just making the obvious fix, but in the long
term I'm convinced it pays off in saving you fixing the same bug twice.
And sometimes writing the test first helps you work out what you should
be fixing.

So here is how I did this one step by step:

 - decide you are going to write a test
 - find the place where this ought to live - there's something called
   tests/test_options.py; that looks likely
 - see there are existing tests which are in the same general area
   (which is nice) and they can be adapted
 - add a new fairly trivial test for the behaviour we want, and see that
   it fails
 - add the fix from robert
 - now it passes

The ultimate reason we want this is probably to say 'bzr merge -', so
you might think we should add a blackbox for that.  But that test will
be slower, and generally less tidy and precise than just checking the
particular behaviour we care about here.  Also we can't add that test
until we've done all the other things to read from stdin, and it's
better to just add a small test first.  I did try "bzr diff -" by hand,
which fails, but with an error "no such file" which shows its failing
the right way.

So seeking +1 for


=== modified file 'bzrlib/commands.py'
--- bzrlib/commands.py	2006-06-20 05:32:16 +0000
+++ bzrlib/commands.py	2006-06-26 09:37:47 +0000
@@ -28,6 +28,8 @@
 # TODO: "--profile=cum", to change sort order.  Is there any value in leaving
 # the profile output behind so it can be interactively examined?
 
+# TODO: parse_args should probably be a Command method...
+
 import sys
 import os
 from warnings import warn
@@ -354,7 +356,6 @@
     lookup table, something about the available options, what optargs
     they take, and which commands will accept them.
     """
-    # TODO: chop up this beast; make it a method of the Command
     args = []
     opts = {}
     alias_opts = {}
@@ -365,11 +366,15 @@
     for proc_argv in alias_argv, argv:
         while proc_argv:
             a = proc_argv.pop(0)
-            if argsover:
+            if a == '-':
+                args.append(a)
+                continue
+            elif argsover:
                 args.append(a)
                 continue
             elif a == '--':
-                # We've received a standalone -- No more flags
+                # Means there are no more options; anything with a - after
+                # this point is taken as a regular argument
                 argsover = True
                 continue
             if a[0] == '-':

=== modified file 'bzrlib/tests/test_options.py'
--- bzrlib/tests/test_options.py	2006-06-20 03:30:14 +0000
+++ bzrlib/tests/test_options.py	2006-06-26 09:29:22 +0000
@@ -4,12 +4,14 @@
 from bzrlib.commands import Command, parse_args
 from bzrlib.builtins import cmd_commit, cmd_log, cmd_status
 
-# TODO: might be nice to just parse them into a structured form and test
-# against that, rather than running the whole command.
-
 class OptionTests(TestCase):
     """Command-line option tests"""
 
+    def test_parse_dash(self):
+        """Single dash is accepted as an argument"""
+        r = parse_args(cmd_commit(), ['-'])
+        self.assertEquals(r, (['-'], {}))
+
     def test_parse_args(self):
         """Option parser"""
         eq = self.assertEquals

-- 
Martin