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

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