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

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

[Yann Dirson]

Let's step in as a user of the feature :)

I started to use :/ very recently, and found it very convenient for my
particular use.  I'm running a script which takes a treeish as
argument, which I regularly run on a dev branch on which I am editing
the commits regularly - that is, the commit message rarely change, but
the sha1's do.  Then once I have a working :/ expr, I can just reuse
the line from shell history, saving typing.

Also, for composing the :/ expression, I make use of "git slog|head"
(slog being an alias of mostly "git log --pretty=oneline"), which helps
figuring out a prefix string that does not require too much
keystrokes.  There again, it is a gain compared to grabbing the mouse
to cut+paste a sha1.

Junio wrote:
>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.

Now what I found really strange, is that the ":" in :/ would not act as
it does in the [<treeish>]:<file> syntax.  Allowing to use
[<treeish>]:/<regexp> would not only make it more useful, but also more
consistent with that other syntax.


>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).

Right there is a problem here - even if it works when you mean ".." as
the range separator, it makes it hard to specify a ".." search pattern.

What about using ":/pattern/" as a syntax instead ?  That would also be
close to other familiar stuff - although it would now require quoting
a "/" to include it in the search pattern...

Another issue would be to (de)activate regexp processing (as compared
to just substring or leading-substring behaviour).  Maybe we could use
sed-like trailing modifiers for this - eg. if regexp is made the
default, ":/obj.c/f" would not match "object".  This would also give us
provision to add classical pattern modifiers like /i, and at 1st sight
would still be unambiguous wrt the .. separator.

Does that make sense ?
-- 
Yann

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

[Linus Torvalds]

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)

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

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> 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.

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

[Junio C Hamano]

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.

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

[Linus Torvalds]

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

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

[Jeff King]

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

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

[Johannes Sixt]

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

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

[Linus Torvalds]

On Tue, 6 Apr 2010, Johannes Sixt wrote:
>
> 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

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

[Nazri Ramliy]

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)