[RFC] Using gitrevisions :/search style with other operators

[RFC] Using gitrevisions :/search style with other operators

[Kevin Ballard]

Most of the ways to refer to a git revision can be combined, e.g. I can type
something silly like

  HEAD~2^^2^2~4^{tree}

and this will work as expected.

There is one operator that doesn't allow this at all, and that's the commit
message search (e.g. :/foo). Every character after the :/ is taken to be
part of the regular expression. This is rather unfortunate as it prevents me
from doing something like :/foo^ to refer to the parent of the matching commit.

Does anybody have any opinions on changing this operator? My suggestion would
be to treat a second / as a termination of the pattern and allow operators
(such as ^ or ~<n>) to occur at that point. This would look like

  :/Merge branch 'foo'/^

If you need a / in your message then you can just escape it with a backslash.

A second change to consider is a syntax to specify that you want the 2nd
(or nth) match rather than the first. This could be done like

  :/foo/2

unless someone has a better idea.

And finally, I'd love to be able to specify the ref to search from. The current
behavior is to find the earliest matching commit on any ref, but it seems
I should be able to type something like

  git merge 'origin/pu:/jh\/notes-merge/^2'

in order to grab the jh/notes-merge topic branch from origin/pu and merge
it into HEAD. Currently trying to type something like master:/test will
treat /test as a file path on master, which is something that will never
resolve to a real file due to the leading slash.

Comments/suggestions/critiques are welcome.

-Kevin Ballard

Re: [RFC] Using gitrevisions :/search style with other operators

[Junio C Hamano]

Kevin Ballard <kevin@sb.org> writes:

> There is one operator that doesn't allow this at all, and that's the commit
> message search (e.g. :/foo). Every character after the :/ is taken to be
> part of the regular expression. This is rather unfortunate as it prevents me
> from doing something like :/foo^ to refer to the parent of the matching commit.
>
> Does anybody have any opinions on changing this operator?

If you mean to change what ":/Merge branch 'kb/blame-author-email'" means,
and make my scripts suddenly start breaking, I would be moderately unhappy.

It is fine if we can enhance the extended SHA-1 vocabulary by introducing
a syntax that used to be illegal, hence there is no chance somebody was
using it for other purposes.

I wonder if we can introduce something like ':( ... )' to group things.
E.g.

    $ git log 'HEAD..:( :/Merge branch 'kb/blame-author-email' )^2'

I haven't visited the particular codepath for some time, so I don't know
offhand how involved the change would be, though.

Re: [RFC] Using gitrevisions :/search style with other operators

[Kevin Ballard]

On Nov 8, 2010, at 11:09 AM, Junio C Hamano wrote:

> Kevin Ballard <kevin@sb.org> writes:
> 
>> There is one operator that doesn't allow this at all, and that's the commit
>> message search (e.g. :/foo). Every character after the :/ is taken to be
>> part of the regular expression. This is rather unfortunate as it prevents me
>> from doing something like :/foo^ to refer to the parent of the matching commit.
>> 
>> Does anybody have any opinions on changing this operator?
> 
> If you mean to change what ":/Merge branch 'kb/blame-author-email'" means,
> and make my scripts suddenly start breaking, I would be moderately unhappy.

Ah yes, I suppose you do have a common case it would break.

> It is fine if we can enhance the extended SHA-1 vocabulary by introducing
> a syntax that used to be illegal, hence there is no chance somebody was
> using it for other purposes.
> 
> I wonder if we can introduce something like ':( ... )' to group things.
> E.g.
> 
>    $ git log 'HEAD..:( :/Merge branch 'kb/blame-author-email' )^2'
> 
> I haven't visited the particular codepath for some time, so I don't know
> offhand how involved the change would be, though.

Interesting idea. It certainly solves the problem of being able to embed it
within other operations (though you do then have to worry about escaping any
embedded close-parens in the search), though it does mean my suggestion for
being able to select the 2nd (or nth) match won't work. When I have the time
I'll start taking a look at the code.

Any comments on my suggestion to specify the ref to search from, e.g.
origin/pu:/"Merge 'kb/blame-author-email'"? As I stated before, I believe
this syntax is currently-unused (as it is interpreted as a file lookup, but
file paths starting with / will never resolve to anything in testing),
but I want to make sure I'm not overlooking something.

-Kevin Ballard

Re: [RFC] Using gitrevisions :/search style with other operators

[Jeff King]

On Mon, Nov 08, 2010 at 02:11:54PM -0800, Kevin Ballard wrote:

> >    $ git log 'HEAD..:( :/Merge branch 'kb/blame-author-email' )^2'
> > 
> > I haven't visited the particular codepath for some time, so I don't know
> > offhand how involved the change would be, though.
> 
> Interesting idea. It certainly solves the problem of being able to embed it
> within other operations (though you do then have to worry about escaping any
> embedded close-parens in the search), though it does mean my suggestion for
> being able to select the 2nd (or nth) match won't work. When I have the time
> I'll start taking a look at the code.
> 
> Any comments on my suggestion to specify the ref to search from, e.g.
> origin/pu:/"Merge 'kb/blame-author-email'"? As I stated before, I believe

It seems to me the natural way to do that would be to use our existing
generic "start at this ref and follow some chain" syntax, which is
ref^{foo}. For example: origin/pu^{:Merge 'kb/blame-author-email'}.

We already use it for "follow tree link" and "peel tag objects", and it
is syntactically similar to the usual "follow the ancestry chain" syntax
("ref^"). And I believe it should be unambiguous, as nothing that could
go in ^{} could start with ":". And presumably one would have to
backslash-escape embedded closing curly braces.

We also have ref@{upstream}. The analogue here would be
origin/pu@{:Merge 'kb/blame-author-email'}.

I don't recall whether we put any conscious thought into ref@{upstream}
versus ref^{upstream}. Certainly the former implies to me finding a
particular starting point (as we do for reflogs), and the latter implies
walking object graph (either by parent ancestry links, or through
dereferencing tag and commit pointers). But I may just be rationalizing
after the fact. :)

Certainly this particular case seems to me to be more like "^" or "~";
you are walking the ancestry tree not a specific number of steps, but
until you see a particular subject.

-Peff

Re: [RFC] Using gitrevisions :/search style with other operators

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

>> Any comments on my suggestion to specify the ref to search from, e.g.
>> origin/pu:/"Merge 'kb/blame-author-email'"? As I stated before, I believe
>
> It seems to me the natural way to do that would be to use our existing
> generic "start at this ref and follow some chain" syntax, which is
> ref^{foo}. For example: origin/pu^{:Merge 'kb/blame-author-email'}.
>
> We already use it for "follow tree link" and "peel tag objects", and it
> is syntactically similar to the usual "follow the ancestry chain" syntax
> ("ref^"). And I believe it should be unambiguous, as nothing that could
> go in ^{} could start with ":". And presumably one would have to
> backslash-escape embedded closing curly braces.

Yeah, that may make more sense but I think the operation specifier should
be '/', not ':', no?

We chose ':' as a "funky SHA-1 expression introducer" that can be followed
by other letters to specify what kind of funkiness is being summoned (in
the case of ':/string', you are asking for "search" funkiness), so that we
can extend it with different kind of funkiness later.  That is where my
earlier ':(...)' came from.

> We also have ref@{upstream}. The analogue here would be
> origin/pu@{:Merge 'kb/blame-author-email'}.
>
> I don't recall whether we put any conscious thought into ref@{upstream}
> versus ref^{upstream}.

I think that is because it is nonsensical to say sha1@{u} for an arbitrary
SHA-1 expression (i.e. the operator only makes sense for a branch ref we
can find branch.*.merge for).  So from that perspective, you are right
that "Start from this commit and find an ancestor whose log message
contains this string" goes well with "sha1^{/some string}"

Re: [RFC] Using gitrevisions :/search style with other operators

[Jeff King]

On Tue, Nov 09, 2010 at 07:59:03AM -0800, Junio C Hamano wrote:

> > It seems to me the natural way to do that would be to use our existing
> > generic "start at this ref and follow some chain" syntax, which is
> > ref^{foo}. For example: origin/pu^{:Merge 'kb/blame-author-email'}.
> 
> Yeah, that may make more sense but I think the operation specifier should
> be '/', not ':', no?

Yeah, I don't know what I was thinking when I wrote the text above. It
should definitely be origin/pu^{/Merge...}.

-Peff

Re: [RFC] Using gitrevisions :/search style with other operators

[Yann Dirson]

Jeff wrote:
> It seems to me the natural way to do that would be to use our existing
> generic "start at this ref and follow some chain" syntax, which is
> ref^{foo}. For example: origin/pu^{:Merge 'kb/blame-author-email'}.

We may want to keep the "/" mnemonic (which seems no to conflict
withcurrent use either), rather than the ":" part, with something like
origin/pu^{/Merge 'kb/blame-author-email'}, and keep ":" for future use.

> We also have ref@{upstream}. The analogue here would be
> origin/pu@{:Merge 'kb/blame-author-email'}.

That's somewhat different, it looks like the foo@{...} only applies to
references with name "foo", and not to arbitrary revisions.  Allowing a
search to start from any commit seems more useful here.

Kevin wrote:
> Junio wrote:
> >    $ git log 'HEAD..:( :/Merge branch 'kb/blame-author-email' )^2'
[...]
>
> Interesting idea. It certainly solves the problem of being able to
> embed it within other operations (though you do then have to worry
> about escaping any embedded close-parens in the search), though it
> does mean my suggestion for being able to select the 2nd (or nth)
> match won't work.

Syntax like origin/pu^{/Merge 'kb/blame-author-email'}2 would be
somewhat consistent with the commit^2 case, and would seem unambiguous
as well - a bit weird, though.

-- 
Yann Dirson - Bertin Technologies

Re: [RFC] Using gitrevisions :/search style with other operators

[Kevin Ballard]

On Nov 8, 2010, at 11:30 PM, Yann Dirson wrote:

> Kevin wrote:
>> Junio wrote:
>>>   $ git log 'HEAD..:( :/Merge branch 'kb/blame-author-email' )^2'
> [...]
>> 
>> Interesting idea. It certainly solves the problem of being able to
>> embed it within other operations (though you do then have to worry
>> about escaping any embedded close-parens in the search), though it
>> does mean my suggestion for being able to select the 2nd (or nth)
>> match won't work.
> 
> Syntax like origin/pu^{/Merge 'kb/blame-author-email'}2 would be
> somewhat consistent with the commit^2 case, and would seem unambiguous
> as well - a bit weird, though.

This violates the idea that once you reach the end of a ^{} structure,
it resolves to a commit that can then be modified by subsequent operations.

-Kevin Ballard

Re: [RFC] Using gitrevisions :/search style with other operators

[Yann Dirson]

On Tue, 09 Nov 2010 00:06:47 -0800
Kevin Ballard <kevin@sb.org> wrote:

> On Nov 8, 2010, at 11:30 PM, Yann Dirson wrote:
> 
> > Kevin wrote:
> >> Junio wrote:
> >>>   $ git log 'HEAD..:( :/Merge branch 'kb/blame-author-email' )^2'
> > [...]
> >> 
> >> Interesting idea. It certainly solves the problem of being able to
> >> embed it within other operations (though you do then have to worry
> >> about escaping any embedded close-parens in the search), though it
> >> does mean my suggestion for being able to select the 2nd (or nth)
> >> match won't work.
> > 
> > Syntax like origin/pu^{/Merge 'kb/blame-author-email'}2 would be
> > somewhat consistent with the commit^2 case, and would seem
> > unambiguous as well - a bit weird, though.
> 
> This violates the idea that once you reach the end of a ^{} structure,
> it resolves to a commit that can then be modified by subsequent
> operations.

OK, that's kinda related to the "looks weird" issue. 

Another idea: origin/pu^{:2/Merge 'kb/blame-author-email'}
Since the foo^{objecttype} syntax would not care for a count, it is not
a problem, and it keeps provision for using a count with future
operators.  OTOH, it could be a problem if we extend foo^{bar} to
accept "bar" for other things than object types.

-- 
Yann Dirson - Bertin Technologies

Re: [RFC] Using gitrevisions :/search style with other operators

[Junio C Hamano]

Yann Dirson <dirson@bertin.fr> writes:

>> > Syntax like origin/pu^{/Merge 'kb/blame-author-email'}2 would be
> ...
> Another idea: origin/pu^{:2/Merge 'kb/blame-author-email'}

What are these "2"s?

You need to question how you figured out the commit you want is the second
one reachable (in whatever traversal order) from something in the first
place.  Didn't you use "git log --oneline" or something to find that out?
At that point, you have the object name already, so I doubt such a
"counting" feature is of much practical use.

Re: [RFC] Using gitrevisions :/search style with other operators

[Kevin Ballard]

On Nov 9, 2010, at 4:18 PM, Junio C Hamano wrote:

> Yann Dirson <dirson@bertin.fr> writes:
> 
>>>> Syntax like origin/pu^{/Merge 'kb/blame-author-email'}2 would be
>> ...
>> Another idea: origin/pu^{:2/Merge 'kb/blame-author-email'}
> 
> What are these "2"s?
> 
> You need to question how you figured out the commit you want is the second
> one reachable (in whatever traversal order) from something in the first
> place.  Didn't you use "git log --oneline" or something to find that out?
> At that point, you have the object name already, so I doubt such a
> "counting" feature is of much practical use.

The particular case that prompted this for me was I knew I had created two
commits called "WIP", scheduled for renaming later, and I wanted to quickly
look at the contents of the first one. I would have loved to be able to
type something like `git show :/WIP/2`. I suppose this situation may be rare
enough not to bother supporting it in the new syntax. With the new syntax
it will be possible to do something like `git show HEAD^{:/WIP}^^{:/WIP}`, but
that looks awfully awkward.

Another thing to consider - the current :/foo syntax searches for the newest
commit reachable from any ref. Using the ^{} syntax will require specifying
a ref first. I'm not sure this is a problem though, as I'm not really sure why
:/foo searches from all refs to begin with.

-Kevin Ballard

Re: [RFC] Using gitrevisions :/search style with other operators

[Yann Dirson]

On Tue, 09 Nov 2010 16:33:31 -0800
Kevin Ballard <kevin@sb.org> wrote:

> On Nov 9, 2010, at 4:18 PM, Junio C Hamano wrote:
> 
> > Yann Dirson <dirson@bertin.fr> writes:
> > 
> >>>> Syntax like origin/pu^{/Merge 'kb/blame-author-email'}2 would be
> >> ...
> >> Another idea: origin/pu^{:2/Merge 'kb/blame-author-email'}
> > 
> > What are these "2"s?
> > 
> > You need to question how you figured out the commit you want is the
> > second one reachable (in whatever traversal order) from something
> > in the first place.  Didn't you use "git log --oneline" or
> > something to find that out? At that point, you have the object name
> > already, so I doubt such a "counting" feature is of much practical
> > use.

I usually always have a gitk displaying history.  Whereas it shows the
commit summary, it does not show the sha1 for each commit - and even if
it did, my brain is still more comfortable dealing with words than
hashes (though that may arguably be an effect of aging ;)

> The particular case that prompted this for me was I knew I had
> created two commits called "WIP", scheduled for renaming later, and I
> wanted to quickly look at the contents of the first one. I would have
> loved to be able to type something like `git show :/WIP/2`. I suppose
> this situation may be rare enough not to bother supporting it in the
> new syntax. With the new syntax it will be possible to do something
> like `git show HEAD^{:/WIP}^^{:/WIP}`, but that looks awfully awkward.

Another use for counting would be for reflog, to lookup things like
"2nd to last of yesterday's commits" - that could be spelled like
"master^{:2:yesterday}" or similar.  Not sure it's worth it (I hardly
use the @{anything vague} syntax myself, especially because it is so
vague), but that looked similar enough to be mentionned here.

> Another thing to consider - the current :/foo syntax searches for the
> newest commit reachable from any ref. Using the ^{} syntax will
> require specifying a ref first. I'm not sure this is a problem
> though, as I'm not really sure why :/foo searches from all refs to
> begin with.

The syntax could be extended so that ^{whatever} starts looking at
current commit (ie. HEAD), somewhat like @{whatever} looks at reflog for
current branch.

-- 
Yann Dirson - Bertin Technologies

Re: [RFC] Using gitrevisions :/search style with other operators

[Kevin Ballard]

On Nov 9, 2010, at 11:32 PM, Yann Dirson wrote:

>> Another thing to consider - the current :/foo syntax searches for the
>> newest commit reachable from any ref. Using the ^{} syntax will
>> require specifying a ref first. I'm not sure this is a problem
>> though, as I'm not really sure why :/foo searches from all refs to
>> begin with.
> 
> The syntax could be extended so that ^{whatever} starts looking at
> current commit (ie. HEAD), somewhat like @{whatever} looks at reflog for
> current branch.

:/foo doesn't start from the current commit - it searches all refs. However,
making ^{} search all refs if not given one doesn't make sense for any
operator except :/foo, so I don't think it's worth doing

-Kevin Ballard

Re: [RFC] Using gitrevisions :/search style with other operators

[Yann Dirson]

On Tue, 09 Nov 2010 23:46:59 -0800
Kevin Ballard <kevin@sb.org> wrote:

> On Nov 9, 2010, at 11:32 PM, Yann Dirson wrote:
> 
> >> Another thing to consider - the current :/foo syntax searches for
> >> the newest commit reachable from any ref. Using the ^{} syntax will
> >> require specifying a ref first. I'm not sure this is a problem
> >> though, as I'm not really sure why :/foo searches from all refs to
> >> begin with.
> > 
> > The syntax could be extended so that ^{whatever} starts looking at
> > current commit (ie. HEAD), somewhat like @{whatever} looks at
> > reflog for current branch.
> 
> :/foo doesn't start from the current commit - it searches all refs.
> However, making ^{} search all refs if not given one doesn't make
> sense for any operator except :/foo, so I don't think it's worth doing

Yes, that's why I suggested to make it search from HEAD, not from all
refs.

-- 
Yann Dirson - Bertin Technologies

Re: [RFC] Using gitrevisions :/search style with other operators

[Jakub Narebski]

Yann Dirson <dirson@bertin.fr> writes:
> On Tue, 09 Nov 2010 23:46:59 -0800
> Kevin Ballard <kevin@sb.org> wrote:
>> On Nov 9, 2010, at 11:32 PM, Yann Dirson wrote:
>> 
>>>> Another thing to consider - the current :/foo syntax searches for
>>>> the newest commit reachable from any ref. Using the ^{} syntax will
>>>> require specifying a ref first. I'm not sure this is a problem
>>>> though, as I'm not really sure why :/foo searches from all refs to
>>>> begin with.
>>> 
>>> The syntax could be extended so that ^{whatever} starts looking at
>>> current commit (ie. HEAD), somewhat like @{whatever} looks at
>>> reflog for current branch.

The <ref>@<sth> is about reflogs: <ref>@{<n>}, <ref>@{<aproxidate>},
@{-<n>}, <ref>@{upstream} / <ref>@{u}.  Because HEAD has separate
reflog, then @{<sth>} is about current branch reflog (@{-<n>} uses
HEAD reflog, though).  <ref> must be something that has reflog.

The <obj>^<sth> is about dereferencing: <tag>^{<objtype>} and <tag>^{},
which includes following parents <commit>^ and <commit>^<n>.  Then
there are odd <commit-ish>^! (returning range) and <commit-ish>^@
(all parents) but which nevertheless follow this rule.

The <obj>:<sth> is (with single exception of ':/<regexp>') about
selecting subitem (path): <tree-ish>:<path>, [:<stage>]:<path>

>> 
>> :/foo doesn't start from the current commit - it searches all refs.
>> However, making ^{} search all refs if not given one doesn't make
>> sense for any operator except :/foo, so I don't think it's worth doing
> 
> Yes, that's why I suggested to make it search from HEAD, not from all
> refs.

Perhaps '--all^{/foo}'?  Just kidding... I think ;-)


About n-th match: I think that ^{<n>/foo} or ^{:<n>/foo}... or
^{:nth(<n>)/foo} as generic form of e.g. ^{3rd/foo} a la Perl 6 :-)
Or perhaps even full form ^{m:nth(<n>)/foo}... well, perhaps not.

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: [RFC] Using gitrevisions :/search style with other operators

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> The <ref>@<sth> is about reflogs

Wrong.  "@" in the olden days were limited about reflogs but not these
days.  <ref>@<something> is about refs; <objectname>^<operation> is about
objects (most often commits).

Re: [RFC] Using gitrevisions :/search style with other operators

[Jakub Narebski]

On Wed, 10 Nov 2010, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> > The <ref>@<sth> is about reflogs
> 
> Wrong.  "@" in the olden days were limited about reflogs but not these
> days.  <ref>@<something> is about refs; <objectname>^<operation> is about
> objects (most often commits).

Fact.  I even provided example, and didn't notice it: [<ref>]@{upstream}
does not use reflogs, but config.
-- 
Jakub Narebski
Poland

Re: [RFC] Using gitrevisions :/search style with other operators

[Jeff King]

On Tue, Nov 09, 2010 at 08:30:23AM +0100, Yann Dirson wrote:

> Jeff wrote:
> > It seems to me the natural way to do that would be to use our existing
> > generic "start at this ref and follow some chain" syntax, which is
> > ref^{foo}. For example: origin/pu^{:Merge 'kb/blame-author-email'}.
> 
> We may want to keep the "/" mnemonic (which seems no to conflict
> withcurrent use either), rather than the ":" part, with something like
> origin/pu^{/Merge 'kb/blame-author-email'}, and keep ":" for future use.

Yeah, sorry, the ':' thing was just a think-o on my part. It should
definitely be "/".

> > We also have ref@{upstream}. The analogue here would be
> > origin/pu@{:Merge 'kb/blame-author-email'}.
> 
> That's somewhat different, it looks like the foo@{...} only applies to
> references with name "foo", and not to arbitrary revisions.  Allowing a
> search to start from any commit seems more useful here.

Yeah, agreed.

-Peff