Re: [RFC/PATCH 3/3] builtin/show.c: do not prune by pathspec

From: Michael J Gruber <git@drmicha.warpmail.net>
Date: Mar 31, 2011, 12:07 UTC
Message-ID: <4D946E85.9030405@drmicha.warpmail.net>
In-Reply-To: <4D94688B.1070703@viscovery.net>

Johannes Sixt venit, vidit, dixit 31.03.2011 13:42:

Show 32 quoted lines

> Am 3/31/2011 12:58, schrieb Michael J Gruber:
>> Johannes Sixt venit, vidit, dixit 31.03.2011 12:18:
>>> Am 3/31/2011 11:17, schrieb Michael J Gruber:
>>>> By design, "git show commit -- path" is not "git show commit:path", and
>>>> there is no reason to change that. But "git show commit -- path" simply
>>>> returns nothing at all "most of the time" because it prunes by pathspec
>>>> even though it does not walk commits. This is pretty useless.
>>>>
>>>> So, turn off pruning (but keep diff limiting of course) so that "git
>>>> show commit -- path" shows the commit message and the diff that the
>>>> commit introduces to path (filtered by path); only the diff will be
>>>> empty "most of the time".
>>>
>>> How does this interfere with git show --walk commit -- path? Will it now
>>> show all commits instead of just those that changed path?
>>
>> Hmpft,
>>
>> git show --walk origin/master
>> fatal: unrecognized argument: --walk
>>
>> No, that is without my patch ;)
>>
>> In other words: "ENOPARSEOPTS in revision.c", there is no "--walk".
> 
> 8-( Oops sorry, and thanks for doing research that I should have done. At
> first, I thought it's named --walk because we have --no-walk. But the name
> is --do-walk.
> 
> My question is then:
> 
> How does this interfere with git show --do-walk commit -- path?

Uh, didn't know that one. If I had thought we have a consistent interface before...

So, yes, it changes that case also. Note that nothing in git-show[1] hints at the fact that one can use rev-list arguments, especially that one could walk the commits, so it is undocumented behavior which would change.

I would even say that quite generally it is difficult to find cases where prune and no_walk make sense together. One may construe a user who feeds a list of commits to "rev-list --no-walk" and uses it as a filter, returning only those commits which touch a given pathspec. But, I'm not suggesting to change "rev-list --no-walk -- pathspec" or log.

But I do think that "show" would benefit from a better default. An alternative would be to imply --sparse (i.e. dense=0).

BTW: If you look at cmd_log_reflog() you see that already for other
log-like commands we have the problem that we have to override the
log-centric defaults from cmd_log_init(), which means ignoring certain
user specified rev-list and diff-core options!

Michael