[PATCH] Interpret :/<pattern> as a regular expression

Subsystems: the rest

DORMANTno replies

8 messages, 3 authors, 2016-06-15 · open the first message on its own page

[PATCH] Interpret :/<pattern> as a regular expression

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:16

Earlier, Git interpreted the pattern as a strict prefix, which made
the operator unsuited in many cases.

Now, the pattern is interpreted as a regular expression (which does not 
change the behaviour too much, since few onelines contain special regex 
characters), so that you can say

	git diff :/.*^Signed-off-by:.Zack.Brown

to see the diff against the most recent reachable commit which was
signed off by Zack, whose Kernel Cousin I miss very much.

Signed-off-by: Johannes Schindelin <redacted>
---

	For fun, you can see which committer signed himself off:

		git show :/.*^Signed-off:

	*grin*

 sha1_name.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index d9188ed..f1ba194 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -611,6 +611,8 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
 {
 	struct commit_list *list = NULL, *backup = NULL, *l;
 	int retval = -1;
+	regex_t regexp;
+	regmatch_t regmatch[1];
 
 	if (prefix[0] == '!') {
 		if (prefix[1] != '!')
@@ -622,6 +624,8 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
 	for_each_ref(handle_one_ref, &list);
 	for (l = list; l; l = l->next)
 		commit_list_insert(l->item, &backup);
+	if (regcomp(&regexp, prefix, REG_EXTENDED))
+		return error("invalid regexp: %s", prefix);
 	while (list) {
 		char *p;
 		struct commit *commit;
@@ -630,7 +634,9 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
 		parse_object(commit->object.sha1);
 		if (!commit->buffer || !(p = strstr(commit->buffer, "\n\n")))
 			continue;
-		if (!prefixcmp(p + 2, prefix)) {
+		if (!regexec(&regexp, p + 2, 1, regmatch, 0) &&
+				printf("match: %d\n", regmatch[0].rm_so) &&
+				regmatch[0].rm_so == 0) {
 			hashcpy(sha1, commit->object.sha1);
 			retval = 0;
 			break;
@@ -639,6 +645,7 @@ static int get_sha1_oneline(const char *prefix, unsigned char *sha1)
 	free_commit_list(list);
 	for (l = backup; l; l = l->next)
 		clear_commit_marks(l->item, ONELINE_SEEN);
+	regfree(&regexp);
 	return retval;
 }
 
-- 
1.5.2.1.2827.gba84a8-dirty

Re: [PATCH] Interpret :/<pattern> as a regular expression

From: Jeff King <hidden>
Date: 2016-06-15 22:43:16

On Wed, Jun 13, 2007 at 01:50:22AM +0100, Johannes Schindelin wrote:
Earlier, Git interpreted the pattern as a strict prefix, which made
the operator unsuited in many cases.
Thank you for working on this...I really like the :/ concept, but find
myself wishing for a regex all the time. I have been meaning to do it
since you introduced the original. :)

-Peff

Re: [PATCH] Interpret :/<pattern> as a regular expression

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:16

Hi,

On Wed, 13 Jun 2007, Jeff King wrote:
On Wed, Jun 13, 2007 at 01:50:22AM +0100, Johannes Schindelin wrote:
quoted
Earlier, Git interpreted the pattern as a strict prefix, which made
the operator unsuited in many cases.
Thank you for working on this...I really like the :/ concept, but find
myself wishing for a regex all the time. I have been meaning to do it
since you introduced the original. :)
:-) Since you seem comfortable with regular expressions, maybe you can 
help me: I am looking for a pattern which matches _any_ character, and one 
which matches only non-newlines, both with and without REG_NEWLINE. Hmm?

Tia,
Dscho

Re: [PATCH] Interpret :/<pattern> as a regular expression

From: Jeff King <hidden>
Date: 2016-06-15 22:43:16

On Wed, Jun 13, 2007 at 07:54:59PM +0100, Johannes Schindelin wrote:
:-) Since you seem comfortable with regular expressions, maybe you can 
help me: I am looking for a pattern which matches _any_ character, and one 
which matches only non-newlines, both with and without REG_NEWLINE. Hmm?
Without REG_NEWLINE, any character is just '.', but I think you are
stuck with '[^
]' for non-newlines, since POSIX makes no provisions for quoting the
newline (I just skimmed through POSIX chapter 9, and I didn't see
anything useful).

With REG_NEWLINE, non-newlines is of course '.'. Matching both is tricky
without using extended regular expressions (where you could just do '.|
'). In fact, I have been playing with it for a few minutes and I can't
seem to find a good way, since you really want to represent '.' _inside_
a bracketed alternation sequence. But I don't think there's a character
class for "everything".

I think this would be much easier with pcre, but ISTR some opposition to
that a few months back.

So that's probably not very helpful to you, but at least you have
confirmation from one other person that the answer isn't totally
obvious. :)

-Peff

Re: [PATCH] Interpret :/<pattern> as a regular expression

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:16

Hi,

On Wed, 13 Jun 2007, Jeff King wrote:
On Wed, Jun 13, 2007 at 07:54:59PM +0100, Johannes Schindelin wrote:
quoted
:-) Since you seem comfortable with regular expressions, maybe you can 
help me: I am looking for a pattern which matches _any_ character, and one 
which matches only non-newlines, both with and without REG_NEWLINE. Hmm?
Without REG_NEWLINE, any character is just '.', but I think you are
stuck with '[^
]' for non-newlines, since POSIX makes no provisions for quoting the
newline (I just skimmed through POSIX chapter 9, and I didn't see
anything useful).

With REG_NEWLINE, non-newlines is of course '.'. Matching both is tricky
without using extended regular expressions (where you could just do '.|
'). In fact, I have been playing with it for a few minutes and I can't
seem to find a good way, since you really want to represent '.' _inside_
a bracketed alternation sequence. But I don't think there's a character
class for "everything".

I think this would be much easier with pcre, but ISTR some opposition to
that a few months back.
Actually, that's funny. Yesterday, I repeated my claim that pcre is 
slow on IRC, and Sam Villain on IRC accused me of trolling. But as you can 
see from my postings on this list ($gmane/41682), you can see that _I_ had 
numbers to back up my claim.

So no, I think pcre is just not worth it.
So that's probably not very helpful to you, but at least you have
confirmation from one other person that the answer isn't totally
obvious. :)
That confirmation is at least some consolation to me :-)

Ciao,
Dscho "who is not here to teach, but to learn"

Re: [PATCH] Interpret :/<pattern> as a regular expression

From: Sam Vilain <hidden>
Date: 2016-06-15 22:43:16

Johannes Schindelin wrote:
Actually, that's funny. Yesterday, I repeated my claim that pcre is 
slow on IRC, and Sam Villain on IRC accused me of trolling. But as you can 
see from my postings on this list ($gmane/41682), you can see that _I_ had 
numbers to back up my claim.

So no, I think pcre is just not worth it.
  
A strange thing to conclude from your figures, which show pcre as the
fastest out of several libraries that you tested.

Your figures show exactly what I was saying on IRC - that a DFA
(external grep) vs NFA engine (most regex libraries) is inherently
faster. The paper I linked to, specially selected as I had previously
read a significant amount of the peer review the paper received,
explained this in detail. The one piece of feedback your numbers got
on-list also mentioned this.

However there is a further flaw in your study. All but one of the
performance tests use an external program, which on a given system may
or may not be faster because of pipeline performance characteristics.
You could improve the quality of the result by using the 'pcregrep'
program as a data point. It might also be worth trying a few more
complex patterns. I suggest reading the paper
(http://swtch.com/~rsc/regexp/regexp1.html) for some background before
repeating the experiment.

Apologies for not reviewing your numbers at the time; it sure is hard to
keep on top of this list. But very interesting that they seem to suggest
pcre would be the best choice from a performance perspective, even
though the figures are very preliminary. Perhaps it is worth pursuing
after all.

Sam.

Re: [PATCH] Interpret :/<pattern> as a regular expression

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:43:16

Hi,

On Thu, 14 Jun 2007, Sam Vilain wrote:
Johannes Schindelin wrote:
quoted
So no, I think pcre is just not worth it.
A strange thing to conclude from your figures, which show pcre as the 
fastest out of several libraries that you tested.
The best of the worse. Yes. An external (!) program was 4x faster than 
pcre. I don't know how to make it more obvious that pcre sucks.

Hth,
Dscho

Re: [PATCH] Interpret :/<pattern> as a regular expression

From: Sam Vilain <hidden>
Date: 2016-06-15 22:43:16

Johannes Schindelin wrote:
quoted
A strange thing to conclude from your figures, which show pcre as the 
fastest out of several libraries that you tested.
    
The best of the worse. Yes. An external (!) program was 4x faster than 
pcre. I don't know how to make it more obvious that pcre sucks.
  
Oh, your position is obvious enough, but it is correct and supportable
by the available evidence? Why not read past the first paragraph of my
post and reply to that.

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