Re: [RFC 2/2] grep: use '/' delimiter for paths

3 messages, 3 authors, 2017-01-20 · open the first message on its own page

Re: [RFC 2/2] grep: use '/' delimiter for paths

From: Junio C Hamano <hidden>
Date: 2017-01-19 19:03:05

Stefan Hajnoczi [off-list ref] writes:
If the tree contains a sub-directory then git-grep(1) output contains a
colon character instead of a path separator:

  $ git grep malloc v2.9.3:t
  v2.9.3:t:test-lib.sh:	setup_malloc_check () {
  $ git show v2.9.3:t:test-lib.sh
  fatal: Path 't:test-lib.sh' does not exist in 'v2.9.3'
I am slightly less negative on this compared to 1/2, but not by a
big margin.  The user wanted to feed a subtree to the command,
instead of doing the more natural

    $ git grep -e malloc v2.9.3 -- t

So again, "contains a colon character" is not coming from what Git
does, but the user gave Git "a colon character" when she didn't have
to.

Having said that, if we wanted to start ignoring what the end-user
said in the initial input like 1/2 and 2/2 does (i.e. "this specific
tree object" as an input), I think the approach to solve for 1/2 and
2/2 should be the same.  I think we should decide to do a slash
instead of a colon, not because v2.9.3: has colon at the end and
v2.9.3:t has colon in it, but because both of these are both bare
tree objects.  The patches presented does not seem to base their
decision on the actual object type but on the textual input, which
seems wrong.

Re: [RFC 2/2] grep: use '/' delimiter for paths

From: Stefan Hajnoczi <stefanha@redhat.com>
Date: 2017-01-20 14:12:26

On Thu, Jan 19, 2017 at 10:54:02AM -0800, Junio C Hamano wrote:
Stefan Hajnoczi [off-list ref] writes:
quoted
If the tree contains a sub-directory then git-grep(1) output contains a
colon character instead of a path separator:

  $ git grep malloc v2.9.3:t
  v2.9.3:t:test-lib.sh:	setup_malloc_check () {
  $ git show v2.9.3:t:test-lib.sh
  fatal: Path 't:test-lib.sh' does not exist in 'v2.9.3'
I am slightly less negative on this compared to 1/2, but not by a
big margin.  The user wanted to feed a subtree to the command,
instead of doing the more natural

    $ git grep -e malloc v2.9.3 -- t
I find <rev>:<path> vs <rev> -- <path> confusing:

            | <rev>:<path>         | <rev> -- <path>
  ----------+----------------------+---------------------
  git grep  | OK                   | OK
  ----------+----------------------+---------------------
  git show  | OK                   | <path> ignored
  ----------+----------------------+---------------------
  git log   | no output            | OK
  ----------+----------------------+---------------------

Neither syntax always does what I expect.  If git show <rev> -- <path>
honored <path> then I could use that syntax consistently.

Sorry for going on a tangent.  Does it seem reasonable to handle <path>
in git-show(1) as a UI convenience?
So again, "contains a colon character" is not coming from what Git
does, but the user gave Git "a colon character" when she didn't have
to.

Having said that, if we wanted to start ignoring what the end-user
said in the initial input like 1/2 and 2/2 does (i.e. "this specific
tree object" as an input), I think the approach to solve for 1/2 and
2/2 should be the same.  I think we should decide to do a slash
instead of a colon, not because v2.9.3: has colon at the end and
v2.9.3:t has colon in it, but because both of these are both bare
tree objects.  The patches presented does not seem to base their
decision on the actual object type but on the textual input, which
seems wrong.
Yes, reparsing the name is ugly and I hoped to get feedback with this
RFC.  Thanks for the quick review!

Re: [RFC 2/2] grep: use '/' delimiter for paths

From: Jeff King <hidden>
Date: 2017-01-20 14:26:43

On Fri, Jan 20, 2017 at 02:12:12PM +0000, Stefan Hajnoczi wrote:
I find <rev>:<path> vs <rev> -- <path> confusing:

            | <rev>:<path>         | <rev> -- <path>
  ----------+----------------------+---------------------
  git grep  | OK                   | OK
  ----------+----------------------+---------------------
  git show  | OK                   | <path> ignored
  ----------+----------------------+---------------------
  git log   | no output            | OK
  ----------+----------------------+---------------------

Neither syntax always does what I expect.  If git show <rev> -- <path>
honored <path> then I could use that syntax consistently.

Sorry for going on a tangent.  Does it seem reasonable to handle <path>
in git-show(1) as a UI convenience?
It's not ignored; just as with git-log, it's a pathspec to limit the
diff. E.g.:

  $ git show --name-status v2.9.3
  ...
  M       Documentation/RelNotes/2.9.3.txt
  M       Documentation/git.txt
  M       GIT-VERSION-GEN

  $ git show --name-status v2.9.3 -- Documentation
  M       Documentation/RelNotes/2.9.3.txt
  M       Documentation/git.txt

That's typically less useful than it is with log (where limiting the
diff also kicks in history simplification and omits some commits
entirely). But it does do something.

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help