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

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 13, 2007, 04:52 UTC
Message-ID: <7vr6ogh2w7.fsf@assigned-by-dhcp.pobox.com>
In-Reply-To: <Pine.LNX.4.64.0706130148080.4059@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 13 quoted lines

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

Show 9 quoted lines

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

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.

Show 11 quoted lines

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

Please lose the debugging printf() before submitting.

Show 8 quoted lines

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