Re: [PATCH 3/6] convert some config callbacks to match_config_key

[Jonathan Nieder <jrnieder@gmail.com>]

Jeff King wrote:

> --- a/convert.c
> +++ b/convert.c
> @@ -465,10 +465,8 @@ static int read_convert_config(const char *var, const char *value, void *cb)
>  	 * External conversion drivers are configured using
>  	 * "filter.<name>.variable".
>  	 */
> -	if (prefixcmp(var, "filter.") || (ep = strrchr(var, '.')) == var + 6)
> +	if (match_config_key(var, "filter", &name, &namelen, &ep) < 0 || !name)
>  		return 0;

Hm, I actually find the preimage more readable here.

I like the idea of having a function to do this, though.  Here are a
couple of ideas for making the meaning obvious again for people like
me:

Rename match_config_key() to something like parse_config_key()?
match_ makes it sound like its main purpose is to match it against a
pattern, but really it is more about decomposing into constituent
parts.

Rename ep to something like 'key' or 'filtertype'?  Without the
explicit string processing, it is not obvious what ep is the end of.

[...]
> --- a/userdiff.c
> +++ b/userdiff.c
> @@ -188,20 +188,13 @@ static struct userdiff_driver *parse_driver(const char *var,
>  		const char *value, const char *type)
>  {
>  	struct userdiff_driver *drv;
> -	const char *dot;
> -	const char *name;
> +	const char *name, *key;
>  	int namelen;
>  
> -	if (prefixcmp(var, "diff."))
> -		return NULL;
> -	dot = strrchr(var, '.');
> -	if (dot == var + 4)
> -		return NULL;
> -	if (strcmp(type, dot+1))
> +	if (match_config_key(var, "diff", &name, &namelen, &key) < 0 ||
> +	    strcmp(type, key))
>  		return NULL;
>  
> -	name = var + 5;
> -	namelen = dot - name;
>  	drv = userdiff_find_by_namelen(name, namelen);

What happens in the !name case?  (Honest question --- I haven't checked.)

Generally I like the cleanup.  Thanks for tasteful patch.

Jonathan