Re: [PATCH] make config --add behave correctly for empty and NULL values

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 19, 2014, 06:03 UTC
Message-ID: <xmqqmwb1vwvs.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20140819051732.GA13765@peff.net>

Jeff King <peff@peff.net> writes:

Show 7 quoted lines

> I just used
>
>   #define CONFIG_REGEX_NONE ((void *)1)
>
> as my magic sentinel value, both for the string and compiled regex
> versions. Adding a bit to the store struct is a lot less disgusting and
> error-prone. So I won't share mine here. :)

Actually, I wrote something like that aloud but did not type it out ;-). Great minds think alike.

We already have some code paths that use ((void *)1) as a special pointer value, so in that sense I would say it is not the end of the world if you added a new one. At the end-user level (i.e. people who write callers to set-multivar-in-file function), I actually like your idea of inventing our own string syntax and parse it at the place where we strip '!' out and remember that the pattern's match status needs to be negated. For example, instead of "a^" (to which I cannot say with confidence that no implementation would match the not-at-the-beginning caret literally), I would not mind if we taught set-multivar-in-file that we use "!*" as a mark to tell "this pattern never matches", and have it assign your "never matches" mark, i.e. (void *)1, to store.value_regex. Then matches() would become

	static int matches(const char *key, const char *value)
        {
        	if (strcmp(key, store.key))
                	return 0; /* not ours */
		if (!store.value_regex)
                	return 1; /* always matches */
		if (store.value_regex == CONFIG_REGEX_NONE)
                	return 0; /* never matches */
		return store.do_not_match ^
                	!regexec(store.value_regex, value, 0, NULL, 0);
	}

or something like that, and the ugly "magic" will be localized, which may make it more palatable.