I have just copied over the fixes by Kent Gibson and made this new changeset. <div>On 10/11/06, John Arbash Meinel <<a href="mailto:john@arbash-meinel.com"> john@arbash-meinel.com</a>> wrote:<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">-----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@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 - <a href="http://enigmail.mozdev.org"> http://enigmail.mozdev.org</a> iD8DBQFFLD7uJdeBCYSNAAMRAqhoAKCJ4oyRZec09HMYI3LNJRM4Ut0mzQCfRjgu /kPT3J9AsQjXX6AYcIQbvIs= =mUQt -----END PGP SIGNATURE----- </blockquote></div>