Thread (2 messages) flat view 2 messages, 2 authors, 2016-06-15

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

From: Michael J Gruber <hidden>
Date: 2016-06-15 22:50:58

Junio C Hamano venit, vidit, dixit 02.04.2011 00:59:
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 ;)
    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 [off-list ref]
    Signed-off-by: Junio C Hamano [off-list ref]

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...
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help