[patch] make 'bzr ignore' take multiple arguments (bug 29488)

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Cheuksan Edward Wang wrote:
> OK. Here is the third try.
> 
...

> -    def run(self, name_pattern=None, old_default_rules=None):
> +    def run(self, name_pattern_list=None, old_default_rules=None):
>          from bzrlib.atomicfile import AtomicFile
>          if old_default_rules is not None:
>              # dump the rules and exit
>              for pattern in ignores.OLD_DEFAULTS:
>                  print pattern
>              return
> -        if name_pattern is None:
> -            raise BzrCommandError("ignore requires a NAME_PATTERN")
> +        if name_pattern_list is None or name_pattern_list == []:
> +            raise BzrCommandError("ignore requires at least one NAME_PATTERN or --old-default-rules")

^- Almost there. But now you are wider than 79 characters, so you need
to break this into two lines:
   raise BzrCommandError("ignore requires at least one"
                         " NAME_PATTERN or --old-default-rules")

should be about right.

...

> === modified file bzrlib/tests/blackbox/test_ignore.py // last-changed:wang at ubu
> ... ntu-20061010073621-c17937e940897fdc
> --- bzrlib/tests/blackbox/test_ignore.py
> +++ bzrlib/tests/blackbox/test_ignore.py
> @@ -74,6 +74,11 @@
>          self.assertEquals(self.capture('unknowns'), '')
>          self.assertEquals(file('.bzrignore', 'rU').read(), '*.blah\ngarh\n')

v- 3 small problems with this one

1) self.capture('unknowns') isn't doing anything here, because the
unknown list hasn't changed.

2) self.check_file_contents() is a better function to call, rather than
.read() and assertEquals(). I realize you were just copying the other
call, but we are trying to avoid file(...).read() or file(...).write(),
because they can sometimes leave open file handles on Windows, which
means we can't clean up properly.

3) It really should be a separate test, rather than adding something to
this test.

> +        # 'ignore' works with multiple arguments
> +        self.runbzr('ignore a b c')
> +        self.assertEquals(self.capture('unknowns'), '')
> +        self.assertEquals(file('.bzrignore', 'rU').read(), '*.blah\ngarh\na\nb\nc\n')
> +        
>      def test_ignore_old_defaults(self):
>          out, err = self.run_bzr('ignore', '--old-default-rules')
>          self.assertContainsRe(out, 'CVS')

Other than the wrapping, nothing really blocks this being merged. I'm
just trying to introduce you to some of the 'recommendations' that we
try to follow. (A lot of this is covered in HACKING, but certainly not
everything (and probably not as much as should be there).

Also, I think we want to change AtomicFile(..., 'wt') to AtomicFile(...,
'wb').

I realize that the file is human editable, so we might try to be nice
for the platform, but I don't think we want to have the line endings
change because a new ignore was added on a different platform. And we
are trying to avoid recommending directly editing the file, since we
might switch to moving it away from being a directly versioned file.

I mostly mention it here, because it means your test doesn't need to
open the file with 'rU', because the line endings would always be pure '\n'.
John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFFLD7uJdeBCYSNAAMRAqhoAKCJ4oyRZec09HMYI3LNJRM4Ut0mzQCfRjgu
/kPT3J9AsQjXX6AYcIQbvIs=
=mUQt
-----END PGP SIGNATURE-----