Re: [PATCH 21/25] pickaxe: use PCREv2 for -G and -S

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Wed, Feb 03 2021, Ævar Arnfjörð Bjarmason wrote:

>  void diffcore_pickaxe(struct diff_options *o)
>  {
>  	const char *needle = o->pickaxe;
>  	int opts = o->pickaxe_opts;
> -	regex_t regex, *regexp = NULL;
> -	kwset_t kws = NULL;
> +	struct grep_opt opt;
> +
> +	if (opts & (DIFF_PICKAXE_REGEX | DIFF_PICKAXE_KIND_GS_MASK)) {
> +		grep_init(&opt, the_repository, NULL);
> +#ifdef USE_LIBPCRE2
> +		grep_commit_pattern_type(GREP_PATTERN_TYPE_PCRE, &opt);
> +#else
> +		grep_commit_pattern_type(GREP_PATTERN_TYPE_ERE, &opt);
> +#endif
>  
> -	if (opts & (DIFF_PICKAXE_REGEX | DIFF_PICKAXE_KIND_G)) {
> -		int cflags = REG_EXTENDED | REG_NEWLINE;
>  		if (o->pickaxe_opts & DIFF_PICKAXE_IGNORE_CASE)
> -			cflags |= REG_ICASE;
> -		regcomp_or_die(&regex, needle, cflags);
> -		regexp = &regex;
> -	} else if (opts & DIFF_PICKAXE_KIND_S) {
> -		if (o->pickaxe_opts & DIFF_PICKAXE_IGNORE_CASE &&
> -		    has_non_ascii(needle)) {
> -			struct strbuf sb = STRBUF_INIT;
> -			int cflags = REG_NEWLINE | REG_ICASE;
> -
> -			basic_regex_quote_buf(&sb, needle);
> -			regcomp_or_die(&regex, sb.buf, cflags);
> -			strbuf_release(&sb);
> -			regexp = &regex;
> -		} else {
> -			kws = kwsalloc(o->pickaxe_opts & DIFF_PICKAXE_IGNORE_CASE
> -				       ? tolower_trans_tbl : NULL);
> -			kwsincr(kws, needle, strlen(needle));
> -			kwsprep(kws);
> -		}
> +			opt.ignore_case = 1;
> +		if (opts & DIFF_PICKAXE_KIND_S &&
> +		    !(opts & DIFF_PICKAXE_REGEX))
> +			opt.fixed = 1;
> +
> +		append_grep_pattern(&opt, needle, "diffcore-pickaxe", 0, GREP_PATTERN);
> +		compile_grep_patterns(&opt);
>  	}
>  
> -	pickaxe(&diff_queued_diff, o, regexp, kws,
> +	pickaxe(&diff_queued_diff, o, &opt,
>  		(opts & DIFF_PICKAXE_KIND_G) ? diff_grep : has_changes);
>  
> -	if (regexp)
> -		regfree(regexp);
> -	if (kws)
> -		kwsfree(kws);
> +	if (opts & ~DIFF_PICKAXE_KIND_OBJFIND)
> +		free_grep_patterns(&opt);
> +
>  	return;
>  }


There's a bug here where now different things are dispatched to either
the -S or -G codepath wrongly, I've fixed it in my local version.

Anyway, it's interesting between this and the -U0 change that we have
little/no coverage for some/all of this. I'm trying to address that in
preceding patches in v2.