[PATCH 3 of 8] Add filesystem path to dirstate.statwalk return value

Paul Moore p.f.moore at gmail.com
Thu May 1 16:18:33 CDT 2008


2008/5/1 Matt Mackall <mpm at selenic.com>:
>  Guess what? I've always thought this pattern of passing a ton of
>  arguments and getting back a ton of results was horrible,

I hate it too - I found it pretty much impossible to get my head round
what's going on here :-(

But having said that, I was sufficiently unsure of what was going on,
that I did "the simplest thing that could work". Well, no - my first
attempt was a hack in the caller, and you advised pushing the fspath
stuff down to statwalk - which is the right thing to do, but was
reaching the limits of what I dared touch without breaking stuff. If I
hadn't been able to get the test suite working on Windows, I'd never
have dared do this.

> so I've been working on changing it. This pattern now looks like this in my repo:
>
>  m = cmdutil.match(repo, pats, opts)
>  for abs in repo.walk(m):
>     if m.exact(abs):
>     ...
>
>  ..so I'm afraid this is an untimely step in the wrong direction.

I'm glad that the code has been simplified, but I'm guessing it means
my work has been wasted :-( Bah, that really wasn't what I wanted to
hear.

>  Also, I think it's probably a mistake to return both abs and fsabs
>  anyway. It seems if we're walking the filesystem, we should only return
>  the case on the filesystem and if we're walking the manifest, we should
>  only return the case we find there.

The trouble is that there are 2 things we need, the name the user
passed and the name we actually found (think hg add a when the file is
actually A). But we can't blindly use the argument the user passed,
because it might have been a pattern, or even implicit (hg add a* or
just hg add). So the simple way of dealing with this is to catch the
user's argument after it's filtered down through the pattern matching
layers, and pass it back up to the user, alongside the actual form
found - that's essentially the distinction between the abs and fsabs
return values. Look in cmdutil.addremove to see a case where I use
both.

Of course, looking at the new pattern you show above, I can get at the
matched results before they get passed to walk, so in that form, I
*don't* need to pass abs back up and can just return fsabs.

I wish I'd known you were working on this refactoring when I started
looking at statwalk :-( Is the code available anywhere public?

Paul.


More information about the Mercurial-devel mailing list