threads / discuss / 23348

[RFD} Use regex's in :/ revision naming machinery

Subject: [RFD} Use regex's in :/ revision naming machinery

## tl;dr

8 messages between Apr 5, 2010 and Apr 7, 2010.

replies: 7people: 5as markdown or json

Linus Torvalds· Apr 5, 2010, 23:00 UTC · lore

So this is very much debatable, but I'm including a patch for the discussion to start.

I never use ':/', and part of it is that it's so horribly cumbersome. I'd _like_ to use it to do things like 

	gitk :/slabh..

which would (right now) show what happened since I merged the 'slabh' branch. But that doesn't work, I would need to write it as

	gitk :/"Merge branch 'slabh'"..

at which point it really isn't all that useful any more - in order to figure out the string I need to use, I had to look up the commit, so the whole :/ format was useless. I might as well just have used the SHA1 value instead.

The thing is, I'm not quite sure _who_ uses :/ as things are now, so maybe there is some reason for that incredibly annoying interface.

But with this trivial (LARGELY UNTESTED!) patch, I get the behaviour _I_ want, and I can do my simpler version instead.

It also allows me to do silly things like this:
	git show :/'Signed-off-by: Linus'

which shows the last commit that was signed off by me. Is that really useful? Not bloody likely. But it shows how much more flexible the whole notion of "let's grep the commit message" is than having to match exactly the beginning of it.

		Linus

PS. Do note the "largely untested" part. I think this patch is kind of cool, but it might be totally broken. I did run the test-suite on it, and it still says "failed 0", but that's probably more indicative of limited test coverage of :/ than anything else.

---
 sha1_name.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index bf92417..7570f1a 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -692,12 +692,17 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
 	struct commit_list *list = NULL, *backup = NULL, *l;
 	int retval = -1;
 	char *temp_commit_buffer = NULL;
+	regex_t regex;
 
 	if (prefix[0] == '!') {
 		if (prefix[1] != '!')
 			die ("Invalid search pattern: %s", prefix);
 		prefix++;
 	}
+
+	if (regcomp(&regex, prefix, REG_EXTENDED))
+		die("Invalid search pattern: %s", prefix);
+
 	for_each_ref(handle_one_ref, &list);
 	for (l = list; l; l = l->next)
 		commit_list_insert(l->item, &backup);
@@ -721,12 +726,13 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
 		}
 		if (!(p = strstr(p, "\n\n")))
 			continue;
-		if (!prefixcmp(p + 2, prefix)) {
+		if (!regexec(&regex, p + 2, 0, NULL, 0)) {
 			hashcpy(sha1, commit->object.sha1);
 			retval = 0;
 			break;
 		}
 	}
+	regfree(&regex);
 	free(temp_commit_buffer);
 	free_commit_list(list);
 	for (l = backup; l; l = l->next)
Junio C Hamano· Apr 6, 2010, 01:51 UTC · re: Linus Torvalds · lore

Re: [RFD} Use regex's in :/ revision naming machinery

Linus Torvalds <torvalds@linux-foundation.org> writes:
Show 12 quoted lines
> So this is very much debatable, but I'm including a patch for the 
> discussion to start.
>
> I never use ':/', and part of it is that it's so horribly cumbersome. I'd 
> _like_ to use it to do things like 
>
> 	gitk :/slabh..
>
> which would (right now) show what happened since I merged the 'slabh' 
> branch. But that doesn't work, I would need to write it as
>
> 	gitk :/"Merge branch 'slabh'"..

A related but a larger issue is that I suspect this "two-dot" would not work, as the syntax looks for "Merge branch 'slabh'.." (two-dot taken literally).

I also happen to think that the current ':/' is more or less useless because you cannot tell it to start digging from only these branches, and it becomes dice-throwing which commit it would find.

Compared to these, I don't think anybody has much against using regex. I personally am all for it, but I am not sure how much regex makes it more useful.

Junio C Hamano· Apr 6, 2010, 02:08 UTC · re: Junio C Hamano · lore

Re: [RFD} Use regex's in :/ revision naming machinery

Junio C Hamano <gitster@pobox.com> writes:
> A related but a larger issue is that I suspect this "two-dot" would not
> work, as the syntax looks for "Merge branch 'slabh'.." (two-dot taken
> literally).
Heh, what do I know.  This does seem to work.  Sorry for the noise.
Linus Torvalds· Apr 6, 2010, 02:15 UTC · re: Junio C Hamano · lore

Re: [RFD} Use regex's in :/ revision naming machinery

On Mon, 5 Apr 2010, Junio C Hamano wrote:
> 
> A related but a larger issue is that I suspect this "two-dot" would not
> work, as the syntax looks for "Merge branch 'slabh'.." (two-dot taken
> literally).
That part works. We do the dot-dot handling first. But:
> I also happen to think that the current ':/' is more or less useless
> because you cannot tell it to start digging from only these branches, and
> it becomes dice-throwing which commit it would find.

I do agree that :/ would be much better if it just started from HEAD, rather than from any branch. It doesn't matter too much for me, but that's because for the kernel I tend to have just one branch (with random occasional topic-branches, but since I do merging rather than real development, that's the rare case)

> Compared to these, I don't think anybody has much against using regex.  I
> personally am all for it, but I am not sure how much regex makes it more
> useful.

I have to admit that I never use it. The reason I did that silly patch is that today I thought that I _could_ use it, but then my naive

	gitk :/slabh..
thing didn't work, and I wondered why.

Would I actually use it more regularly if it was a regex? And if it had saner semantics wrt branches? I dunno. Maybe my try at using it today was the last time I'd ever try to use it again, regardless.

			Linus
Jeff King· Apr 6, 2010, 04:31 UTC · re: Linus Torvalds · lore

Re: [RFD} Use regex's in :/ revision naming machinery

On Mon, Apr 05, 2010 at 04:00:59PM -0700, Linus Torvalds wrote:
> So this is very much debatable, but I'm including a patch for the 
> discussion to start.

FWIW, this came up quite a while ago and people generally seemed to think it was a good idea, but the patch from Dscho (cc'd) never made it upstream.

The old thread is here:
  http://article.gmane.org/gmane.comp.version-control.git/50010

Personally, I think regex matching makes :/ a bit more usable, though since that thread I have found I don't use :/ all that often (instead needing to pick through the results of --grep looking for the commit in question, at which point I just cut-and-paste the commit sha1). I agree as was suggested elsewhere in the thread that searching from HEAD would help.

-Peff
Johannes Sixt· Apr 6, 2010, 07:55 UTC · re: Linus Torvalds · lore

Re: [RFD} Use regex's in :/ revision naming machinery

Am 4/6/2010 1:00, schrieb Linus Torvalds:
> I never use ':/', and part of it is that it's so horribly cumbersome. I'd 
> _like_ to use it to do things like 
> 
> 	gitk :/slabh..

Rhethoric question: Do you mean history _starting_ at the commit that contains "slabh" or _ending_ at the commit that contains "slabh" followed by two arbitrary characters?

If :/ is pattern-ized in some way, then IMO pattern matching syntax would be more use-friendly than (extended) regular expresssions, particluarly also because the single-character wildcard would be ? and avoid the otherwise overloaded dot.

-- Hannes
Linus Torvalds· Apr 6, 2010, 14:19 UTC · re: Johannes Sixt · lore

Re: [RFD} Use regex's in :/ revision naming machinery

On Tue, 6 Apr 2010, Johannes Sixt wrote:
Show 10 quoted lines
>
> Am 4/6/2010 1:00, schrieb Linus Torvalds:
> > I never use ':/', and part of it is that it's so horribly cumbersome. I'd 
> > _like_ to use it to do things like 
> > 
> > 	gitk :/slabh..
> 
> Rhethoric question: Do you mean history _starting_ at the commit that
> contains "slabh" or _ending_ at the commit that contains "slabh" followed
> by two arbitrary characters?
Ending in the commit that contains "slabh".

The last two dots are removed by the revision handling machinery, _before_ we even see the ':/' thing. See 'handle_revision_arg()', and notice how it searches for ".." and then does "*dotdot = 0".

So the regex is 'slabh'.
> If :/ is pattern-ized in some way, then IMO pattern matching syntax would
> be more use-friendly than (extended) regular expresssions, particluarly
> also because the single-character wildcard would be ? and avoid the
> otherwise overloaded dot.

I do agree that regex'es might contain the invalid sequence "..", but it's fairly unusual, and you can work around it, ie you can do

	gitk :/'slabh(.)(.)of'..

which now that I write it out admittedly looks like you are searching for titties, but hey, sex sells, so we can call it a _feature_.

			Linus
Nazri Ramliy· Apr 7, 2010, 01:04 UTC · re: Linus Torvalds · lore

Re: [RFD} Use regex's in :/ revision naming machinery

On Tue, Apr 6, 2010 at 10:19 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote:

>        gitk :/'slabh(.)(.)of'..
>
> which now that I write it out admittedly looks like you are searching for
> titties, but hey, sex sells, so we can call it a _feature_.
I knew it! You've had this planned out since the early beginnings!
There's a reason why you chose the name 'blob', no?
git grep blob|sed -e 's/blo/boo/g'|less
Hidden features?
Warning: Don't try this at home. You may never read the source the
same way again!
nazri (I'll go back to my cave)

← back to recent threads