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

Johannes Schindelin [off-list ref] writes:

Because you are not extracting any match substring, I do not
think you would need regmatch[] there.

Why EXTENDED?

I think our code prefer traditional regexp by default, but in
places where extended behaviour is truly useful (e.g. grepping
for bulk of text) have command line option to enable extended.
Of course, extended SHA-1 notation has no chance to do "command
line option", but it somehow feels inconsistent.

Also you probably want REG_NEWLINE.

Do we want to detect return value other than REG_NOMATCH from
regexec() when it does not return zero?

Please lose the debugging printf() before submitting.

Also I think you would want to fix get_sha1_oneline() so that it
not to refuse to work without save_commit_buffer.  These are to
parse user supplied strings (it would be crazy for scripts to
throw hundreds of ':/random string' to drive git -- it must come
from the end user), and the user has every right to use this
syntax, if he wants to, to specify the starting point for a
command that deliberately turns off save_commit_buffer to save
memory, because the command knows ithat t will traverse tons of
commits without needing the contents of the commit buffer.

After parse_object(), if commit->buffer is NULL, read the buffer
with read_sha1_file() yourself to look for match (and if you did
so you are also responsible for discarding it yourself).

Hi,

On Tue, 12 Jun 2007, Junio C Hamano wrote:

Johannes Schindelin [off-list ref] writes:

quoted

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] != '!')

Because you are not extracting any match substring, I do not
think you would need regmatch[] there.

I explicitely want to anchor the match at the beginning of the message 
(otherwise, most of the interesting patterns would match a merge pulling 
that commit in), and therefore I have to check where the match was. Alas, 
that's only possible with regmatch.

quoted

@@ -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;

Why EXTENDED?

Sorry. Old habit. Will fix.

Also you probably want REG_NEWLINE.

No. If I look for something in the body of the message (like I showed in 
the two examples), I'd like to say ':/.*Blub'. Of course, you could always 
say

	git diff ':/[ -~
]*Blub'

with REG_NEWLINE, to match newlines also. But frankly, it is much more 
often that I want to match something in the whole message than just in the 
oneline. And if I _do_ want the match only in the oneline, I can still go 
and do

	git diff ':/[^
]*Blub'

when REG_NEWLINE is disabled. If you can teach me a better way to do both 
things, matching in the oneline _or_ matching the whole message, _with_ 
REG_NEWLINE, I'll gladly change it, and provide an example in the commit 
message as well as the documentation for equally clueless subjects as me.

quoted

@@ -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;

Do we want to detect return value other than REG_NOMATCH from
regexec() when it does not return zero?

I am not well versed in the multitude of POSIX and other standards we have 
on this planet, therefore I just read my man page. And it says, quote:

	regexec() returns zero for a successful match or REG_NOMATCH for 
	failure.

Tertium non datur. At least according to my man page here. Am I mistaken 
in my assumption (which seems to be somewhat supported from my limited 
reading of the man page) that all errors should be caught at regcomp() 
time?

Please lose the debugging printf() before submitting.

Ouch. Sorry.

quoted

@@ -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;
 }
 

Also I think you would want to fix get_sha1_oneline() so that it
not to refuse to work without save_commit_buffer.

You're right, will fix.

I'll resubmit shortly. However, feel free to enlighten me with insights 
into working _with_ REG_NEWLINE, and I'll gladly submit another version of 
the patch with REG_NEWLINE enabled, along with the additions to the 
documentation.

Thanks,
Dscho

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

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, on the whole
message, 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.

If you want to match just the oneline, but with a regular expression,
say something like

	git diff ':/[^
]*intelligent'

Since it makes more sense to match the beginning of a message (otherwise,
a pattern like ':/git-gui: Improve' would match the _merge_ commit, 
pulling in the commit you are likely to want), the implementation uses the 
regmatch parameter of regexec() to anchor the pattern there.

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

	Anyone who can teach me how to match / not-match a newline
	using a more elegant syntax, please do so, by all means.

 Documentation/git-rev-parse.txt |    6 +++---
 sha1_name.c                     |   31 +++++++++++++++++++++++++------
 2 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 6380676..56e1561 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt

@@ -194,9 +194,9 @@ blobs contained in a commit.
   found.
 
 * A colon, followed by a slash, followed by a text: this names
-  a commit whose commit message starts with the specified text.
-  This name returns the youngest matching commit which is
-  reachable from any ref.  If the commit message starts with a
+  a commit whose commit message starts with the specified regular
+  expression.  This name returns the youngest matching commit which
+  is reachable from any ref.  If the commit message starts with a
   '!', you have to repeat that;  the special sequence ':/!',
   followed by something else than '!' is reserved for now.
 

diff --git a/sha1_name.c b/sha1_name.c
index d9188ed..988d599 100644
--- a/sha1_name.c
+++ b/sha1_name.c

@@ -599,8 +599,8 @@ static int handle_one_ref(const char *path,
 
 /*
  * This interprets names like ':/Initial revision of "git"' by searching
- * through history and returning the first commit whose message starts
- * with the given string.
+ * through history and returning the first commit whose message matches
+ * the given regular expression.
  *
  * For future extension, ':/!' is reserved. If you want to match a message
  * beginning with a '!', you have to repeat the exclamation mark.

@@ -611,34 +611,53 @@ 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];
+	char *temp_commit_buffer = NULL;
 
 	if (prefix[0] == '!') {
 		if (prefix[1] != '!')
 			die ("Invalid search pattern: %s", prefix);
 		prefix++;
 	}
-	if (!save_commit_buffer)
-		return error("Could not expand oneline-name.");
 	for_each_ref(handle_one_ref, &list);
 	for (l = list; l; l = l->next)
 		commit_list_insert(l->item, &backup);
+	if (regcomp(&regexp, prefix, 0))
+		return error("invalid regexp: %s", prefix);
 	while (list) {
 		char *p;
 		struct commit *commit;
+		enum object_type type;
+		unsigned long size;
 
 		commit = pop_most_recent_commit(&list, ONELINE_SEEN);
 		parse_object(commit->object.sha1);
-		if (!commit->buffer || !(p = strstr(commit->buffer, "\n\n")))
+		if (temp_commit_buffer)
+			free(temp_commit_buffer);
+		if (commit->buffer)
+			p = commit->buffer;
+		else {
+			p = read_sha1_file(commit->object.sha1, &type, &size);
+			if (!p)
+				continue;
+			temp_commit_buffer = p;
+		}
+		if (!(p = strstr(p, "\n\n")))
 			continue;
-		if (!prefixcmp(p + 2, prefix)) {
+		if (!regexec(&regexp, p + 2, 1, regmatch, 0) &&
+				regmatch[0].rm_so == 0) {
 			hashcpy(sha1, commit->object.sha1);
 			retval = 0;
 			break;
 		}
 	}
+	if (temp_commit_buffer)
+		free(temp_commit_buffer);
 	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

Hi,

I don't know what happened to my subject in the mail I'm replying to... 
Sorry.

Ciao,
Dscho