[PATCH] addremove: print relative paths when called with -I/-X

[Matt Harbison]

On Thu, Dec 4, 2014 at 12:40 PM, Martin von Zweigbergk <
martinvonz at google.com> wrote:

>
>
> [snip]
>>
>>
>>>   I didn't run the
>>> >> tests to see if that breaks anything.  (I'm not sure why it didn't
>>> also
>>> >> list the directory I was in as a relpath too.)
>>> >>
>>> >
>>> > Without having spent much thought to it, does 'not m.always()' work
>>> > better?
>>> > I ask because that's what it seemed like status and locate wanted when
>>> I
>>> > checked. It's just mostly the same for addremove since exact() was
>>> > already
>>> > discounted.
>>>
>>> I'm not sure.  My concern is that largefiles creates matchers with the
>>> _always field set to False.  See composenormalfilematcher() in
>>> largefiles/overrides.py.  I haven't sent the second half of the series
>>> yet, but that matcher will be passed to this code, and it basically
>>> eliminates the conditional when it is hardcoded to False.  It makes sense
>>> that the largefiles matcher is hardcoded, since it is excluding some
>>> files.  So this idea seems close, but a step in the wrong direction.
>>>
>>> It seems surprising to me that a list called patterns can be given to the
>>> matcher, and then the matcher reports that no patterns were given.  Maybe
>>> it is just an unfortunate similarity in the names, but it seems like that
>>> needs to be fixed (or better documented what the differences are).
>>>
>>> --Matt
>>>
>>
>> Does it seem fair to say that we want to use relative paths in the UI in
>> a few cases (addremove, status, locate) if the user has not restricted the
>> paths in any way? If it is, it seems like we would need a flag on the
>> matcher that is disconnected from what the matcher actually matches. That
>> might be your _anyfiles, but modified to be "bool(patterns or include or
>> exclude)" and maybe renamed to _userelativepathsinui (but shortened). *sigh*
>>
>
> But in that case it seems better to just keep the knowledge outside of
> matcher. Does the need to pass it around in the matcher arise from subrepo
> use cases? I guess I didn't get that part from your patches.
>

I'm not sure I understand.  I replaced the patterns list with a matcher,
because it seems like a matcher is more functional for subrepos.  E.g. the
pattern list isn't narrowed if I kept passing it along to subrepos, so it
is both (kind of) redundant with the matcher and wrong inside a subrepo.
The only thing it is good for at that point is to decide abs or rel.

It might be nice to move the ((pats and rel) or abs) logic into a matcher
function to hide that logic, and then however we store that info doesn't
have to be public on the matcher.  I just don't know if the pattern is used
enough to justify that move, and nobody chimed in about why some methods
use abs vs rel and others don't.  Passing along an external 'what print
style should is use' parameter seems ugly to me.

--Matt
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20141204/69851287/attachment.html>