[PATCH 2 of 2] Get add and locate to use new repo and dirstate walk code

[Bryan O'Sullivan]

[Thomas, I assume you meant your reply to go to the list?]

On Mon, 2005-07-18 at 17:30 +0200, Thomas Arendsen Hein wrote:

> > +    glob:*.c       any name ending in ".c" in the current directory
> > +    *.c            any name ending in ".c" in the current directory
> > +    **.c           any name ending in ".c" in the current directory, or
> > +                   any subdirectory
> > +    foo/**.c       any name ending in ".c" in the directory foo, or any
> > +                   subdirectory

> While this sounds correct, I'd expected glob patterns to be
> compatible with .cvsignore, i.e "*.c" would match a C file in any
> directory and ** only needed for the last example.

I have no problem with the contents of .hgignore being compatible
with .cvsignore.

However, I am adamantly opposed to the idea that patterns on the command
line should match in anything other than the current directory.

If I type "hg rm '*.c'", I expect it to delete any file ending in ".c"
in the current directory, and no other.  Ditto for "hg rm 'foo/*.c'",
which should not delete files like "bar/foo/baz.c".

This is true for every other command, with the exception of locate.


> > +    Regexp examples:
> > +
> > +    re:.*\.c$      any name ending in ".c", anywhere in the repsitory
> 
> This is not a good example, since the ".*" isn't needed (or is there
> a '^' implied?)

There is indeed a ^ implied.  If you can think of a more realistic
example, I'm all for it :)

> > -def add(ui, repo, file1, *files):
> > +def add(ui, repo, *pats, **opts):
> >      '''add the specified files on the next commit'''
> > -    repo.add(relpath(repo, (file1,) + files))
> 
> def add(ui, repo, pat1, *pats, **opts):
>     pats = (pat1,) + pats
>     ...
> 
> This makes sure that at least one filename is given to the add
> command. The corresponding TODO entry is:
> "commands.py: number of args too much magic" ;-)
> 
> Or should 'hg add' work like 'hg addremove' just without removing?

That is how it works now, with my changes.  I'm not very happy with the
behaviour, but that's my recollection of how people wanted it to behave.

> >  table = {
> > -    "^add": (add, [], "hg add [files]"),
> > +    "^add": (add,
> > +             [('I', 'include', [], 'include path in search'),
> > +              ('X', 'exclude', [], 'exclude path from search')],
> > +             "hg add [options] [files]"),
> 
> Your patch seems not to be based on the current tip, as this would
> have "hg add FILE..." instead of "hg add [files]".

It's based on the tip of a few days ago.  I can forward-port it as
necessary.

> > -          ('i', 'include', [], 'include path in search'),
> > +          ('I', 'include', [], 'include path in search'),
> > -          ('x', 'exclude', [], 'exclude path from search')],
> > +          ('X', 'exclude', [], 'exclude path from search')],
> 
> Is there a specific reason for the case change? But I like it.

Yes, some other commands (such as grep) really want to be able to use
lower-case "-i".  By using "-I" and "-X", I think we have a better
chance of every command using consistent option names for include and
exclude.

> Hm, this isn't in the current tip revision. Maybe something from one
> of your other patches?

Yes, from 1.

> > -            elif self.dirstate.state(f) == 'n':
> > +            elif self.dirstate.state(f) in 'an':
> >                  self.ui.warn("%s already tracked!\n" % f)
> 
> I considered this a feature and not a bug: If you add a file twice
> before committing, there is no error, as it is still added only
> once. Just as 'touch foo; touch foo' isn't an error.

It's not an error, still; it just tells you that it already knows about
the file.  And "touch" does something entirely different when a file
already exists compared to when it doesn't, so it's a bad example :-)

> Escaping of *?[{ with \ should be possible like in the shell.

Sure.  I just copied the Python glob code as it stood.

	<b

-- 
Bryan O'Sullivan <bos at serpentine.com>