git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Michael J Gruber <git@drmicha.warpmail.net>
Cc: git@vger.kernel.org, "Johannes Sixt" <j6t@kdbg.org>,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: Re: [PATCH 4/4] builtin/show: do not prune by pathspec
Date: Fri, 01 Apr 2011 15:59:16 -0700	[thread overview]
Message-ID: <7v7hbdefy3.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <7vwrjdej42.fsf@alter.siamese.dyndns.org> (Junio C. Hamano's message of "Fri, 01 Apr 2011 14:50:53 -0700")

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.
    
    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.

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.

  reply	other threads:[~2011-04-01 22:59 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-31  6:45 Usability improvement request: git show revision -- file Piotr Krukowiecki
2011-03-31  7:50 ` Michael J Gruber
2011-03-31  9:17   ` [PATCH 1/3] t1506: factor out test for "Did you mean..." Michael J Gruber
2011-03-31  9:17     ` [PATCH 2/3] sha1_name: Suggest commit:./file for path in subdir Michael J Gruber
2011-03-31 19:26       ` Junio C Hamano
2011-04-01  6:52         ` Michael J Gruber
2011-04-01 19:11           ` Junio C Hamano
2011-03-31  9:17     ` [RFC/PATCH 3/3] builtin/show.c: do not prune by pathspec Michael J Gruber
2011-03-31 10:18       ` Johannes Sixt
2011-03-31 10:58         ` Michael J Gruber
2011-03-31 11:42           ` Johannes Sixt
2011-03-31 12:07             ` Michael J Gruber
2011-03-31 12:50       ` Nguyen Thai Ngoc Duy
2011-03-31 13:26         ` Michael J Gruber
2011-03-31 13:35           ` Nguyen Thai Ngoc Duy
2011-03-31 13:55             ` Michael J Gruber
2011-03-31 19:23           ` Junio C Hamano
2011-04-01  6:46             ` Michael J Gruber
2011-04-01  9:20             ` [PATCH 0/4] reflog, show and command line overrides Michael J Gruber
2011-04-01  9:20               ` [PATCH 1/4] builtin/log.c: separate default and setup of cmd_log_init() Michael J Gruber
2011-04-01  9:20               ` [PATCH 2/4] t/t1411: test reflog with formats Michael J Gruber
2011-04-01  9:20               ` [PATCH 3/4] reflog: fix overriding of command line options Michael J Gruber
2011-04-01  9:20               ` [PATCH 4/4] builtin/show: do not prune by pathspec Michael J Gruber
2011-04-01 21:50                 ` Junio C Hamano
2011-04-01 22:59                   ` Junio C Hamano [this message]
2011-04-03 13:16                     ` Michael J Gruber
2011-04-04 21:49                 ` Junio C Hamano
2011-04-05  6:06                   ` Michael J Gruber
2011-03-31 19:59   ` Usability improvement request: git show revision -- file Piotr Krukowiecki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7v7hbdefy3.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    --cc=j6t@kdbg.org \
    --cc=pclouds@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).