Re: [PATCH 4/4] builtin/show: do not prune by pathspec

From: Michael J Gruber <git@drmicha.warpmail.net>
Date: Apr 3, 2011, 13:16 UTC
Message-ID: <4D98732E.5010709@drmicha.warpmail.net>
In-Reply-To: <7v7hbdefy3.fsf@alter.siamese.dyndns.org>

Junio C Hamano venit, vidit, dixit 02.04.2011 00:59:

Show 12 quoted lines

> I attempted to rewrite the log message in a bit more objective voice like
> this:
> 
>     builtin/show: do not prune by pathspec
>     
>     "git show $commit -- $path" does not show anything for a commit that does
>     not change the $path.  While this may be technically correct, it is somewhat
>     unexpected from the end user's point of view.
>     
>     Unless "show" is used as "log -p", e.g. "git show HEAD~5..", it makes more
>     sense to show at least the log message for commits, even they are
>     uninteresting with respect to $path.

Definitely more diplomatic ;)

Show 13 quoted lines

>     Turn off commit pruning (but keep diff limiting of course) so that the
>     command shows the log message and the diff that the commit introduces to
>     the path.  The diff part may be empty for a given commit that does not
>     touch the path.
>     
>     As an intended side effect, users mistaking "git show commit -- path"
>     for "git show commit:path" are automatically reminded that they asked
>     git to show a commit, not a blob.
>     
>     Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>
>     Signed-off-by: Junio C Hamano <gitster@pobox.com>
> 
> which made me realize that this change does regress for no-walk case.

Well, it's an intended change of behaviour, so the term "regress" is diplomatically problematic...

Show 16 quoted lines

> 
> What if you (or your homegrown tool or alias) are feeding a list of
> candidate commits that may have touched the path, without walking, and are
> expecting them to be filtered?
> 
>     $ git show A B C D -- path
> 
> We used to get a nice output of "git show C -- path" in such a case but
> now the output will be cluttered with the log message from a commit that
> is totally uninteresting with respect to the given path.
> 
> I really wanted to like this patch, because I _very much_ liked the
> "intended" ;-) side effect.
> 
> I am torn.
>

I would probably discard the "tool" aspect (for a porcelain). FWIW, we never even documented that "git show" takes pathspec arguments, and only experienced users know that "git show" has anything to do with "git log" and friends (we mention only the relation to diff-tree).

That being said, we could detect in addition whether there is only one positive rev (and no walk being done) and turn off pruning only in that case. That would still catch users' (mis)expectation about "show rev -- file" but not impact anyone using it as a filter on more than 1 rev. (Personally, I find the distinction between walk and no-walk modes clearer.)

Michael