public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: Stefan Hajnoczi <stefanha@redhat•com>
Cc: git@vger•kernel.org
Subject: Re: [RFC 1/2] grep: only add delimiter if there isn't one already
Date: Fri, 20 Jan 2017 10:16:31 -0800	[thread overview]
Message-ID: <xmqqlgu5y4u8.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20170120135612.GB17499@stefanha-x1.localdomain> (Stefan Hajnoczi's message of "Fri, 20 Jan 2017 13:56:12 +0000")

Stefan Hajnoczi <stefanha@redhat•com> writes:

> v2.9.3::Makefile may convey that the user originally provided v2.9.3:
> but is that actually useful information?

You are either asking a wrong question, or asking a wrong person
(i.e. Git) the question.  The real question is why the user added a
colon at the end, when "v2.9.3" alone would have sufficed.  What did
the user want to get out of giving an extra colon like "v2.9.3:"?

One answer I can think of is that it is a degenerate case of [2/2],
i.e. if "v2.9.3:t" were an appropriate way to look in the subtree
"t" of "v2.9.3", "v2.9.3:" would be the appropriate way to look in
the whole tree of "v2.9.3".

I understand, from your response to my comment in the thread for
[2/2], that the reason why "v2.9.3:t" was used in the example was
because you felt uncomfortable with using pathspec.  

That's superstition.

My only piece of advice to folks who feel that way is to learn Git
more and get comfortable.  You can do neat things like

   $ git grep -e pattern rev -- t ':!t/helper/'

that you cannot do with "rev:t", for example ;-)

All examples we saw so far are the ones that the user used the colon
syntax when it is more natural to express the command without it.  I
am hesitant to see the behaviour of the command changed to appease
such suboptimal use cases if it requires us to discard a bit of
information, when we haven't established it is OK to lose.

There may be a case

 (1) where the colon syntax is the most natural thing to use, AND
     script reading the output that used that syntax is forced to do
     unnecessary work because "git grep" parrots the colon
     literally, instread of being more intelligent about it
     (i.e. omitting or substituting with slash when the input is a
     tree object, not a tree-ish, as discussed in the thread).

 (2) where the colon syntax is the most natural thing, AND script
     reading the output WANTS to see the distinction in the input
     (remember, "git grep" can take more than one input tree).

We haven't seen either of the above two in the discussion, so the
discussion so far is not sufficient to support the change being
proposed in this RFC, which requires that it is safe to assume that
(1) is the only case where the input is a raw tree (or the input
contains a colon) and (2) will never happen.

So I am still mildly negative on the whole thing.

  reply	other threads:[~2017-01-20 18:16 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-19 15:03 [RFC 0/2] grep: make output consistent with revision syntax Stefan Hajnoczi
2017-01-19 15:03 ` [RFC 1/2] grep: only add delimiter if there isn't one already Stefan Hajnoczi
2017-01-19 18:39   ` Junio C Hamano
2017-01-20 13:56     ` Stefan Hajnoczi
2017-01-20 18:16       ` Junio C Hamano [this message]
2017-01-23 13:15         ` Stefan Hajnoczi
2017-01-24 19:07           ` Jakub Narębski
2017-01-24 20:48             ` Philip Oakley
2017-01-19 15:03 ` [RFC 2/2] grep: use '/' delimiter for paths Stefan Hajnoczi
2017-01-19 18:29   ` Brandon Williams
2017-01-20 14:17     ` Stefan Hajnoczi
2017-01-19 18:54   ` Junio C Hamano
2017-01-20 14:12     ` Stefan Hajnoczi
2017-01-20 14:19       ` Jeff King
2017-01-20 22:56         ` Junio C Hamano
2017-01-23 13:29           ` Stefan Hajnoczi
2017-01-24 17:18             ` Phil Hord
2017-01-19 16:59 ` [RFC 0/2] grep: make output consistent with revision syntax Jeff King
2017-01-19 18:26   ` Brandon Williams
2017-01-20 14:18   ` Stefan Hajnoczi
2017-01-20 14:32     ` Jeff King

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

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

  git send-email \
    --in-reply-to=xmqqlgu5y4u8.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=stefanha@redhat$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox