# [RFC] Using gitrevisions :/search style with other operators 6 messages from 2010-11-05 to 2010-11-09. Participants: Kevin Ballard, Junio C Hamano, Jeff King. Thread: https://gitlist.dev/t/25652 ## Kevin Ballard, 2010-11-05 22:38 Subject: [RFC] Using gitrevisions :/search style with other operators Message-ID: <9D675671-693D-4B59-AF2A-0EFE4C537362@sb.org> URL: https://gitlist.dev/e/9D675671-693D-4B59-AF2A-0EFE4C537362%40sb.org ``` 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 ~) 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 ``` ## Junio C Hamano, 2010-11-08 19:09 Subject: Re: [RFC] Using gitrevisions :/search style with other operators Message-ID: <7vpqufpr7f.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vpqufpr7f.fsf%40alter.siamese.dyndns.org In-Reply-To: <9D675671-693D-4B59-AF2A-0EFE4C537362@sb.org> ``` Kevin Ballard 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. ``` ## Kevin Ballard, 2010-11-08 22:11 Subject: Re: [RFC] Using gitrevisions :/search style with other operators Message-ID: <4F75AA0C-B68A-4ABB-B67A-D7F62D989655@sb.org> URL: https://gitlist.dev/e/4F75AA0C-B68A-4ABB-B67A-D7F62D989655%40sb.org In-Reply-To: <7vpqufpr7f.fsf@alter.siamese.dyndns.org> ``` On Nov 8, 2010, at 11:09 AM, Junio C Hamano wrote: > Kevin Ballard 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 ``` ## Jeff King, 2010-11-09 05:16 Subject: Re: [RFC] Using gitrevisions :/search style with other operators Message-ID: <20101109051631.GA19867@sigill.intra.peff.net> URL: https://gitlist.dev/e/20101109051631.GA19867%40sigill.intra.peff.net In-Reply-To: <4F75AA0C-B68A-4ABB-B67A-D7F62D989655@sb.org> ``` 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 ``` ## Junio C Hamano, 2010-11-09 15:59 Subject: Re: [RFC] Using gitrevisions :/search style with other operators Message-ID: <7v4obqo5dk.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v4obqo5dk.fsf%40alter.siamese.dyndns.org In-Reply-To: <20101109051631.GA19867@sigill.intra.peff.net> ``` Jeff King 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}" ``` ## Jeff King, 2010-11-09 16:08 Subject: Re: [RFC] Using gitrevisions :/search style with other operators Message-ID: <20101109160833.GA24975@sigill.intra.peff.net> URL: https://gitlist.dev/e/20101109160833.GA24975%40sigill.intra.peff.net In-Reply-To: <7v4obqo5dk.fsf@alter.siamese.dyndns.org> ``` 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 ```