Re: [PATCH 1/3] git-compat-util: introduce skip_to_opt_val()

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 4, 2017, 13:35 UTC
Message-ID: <xmqqpo7uobuc.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CAP8UFD0UH62fyLLAx8QP37L22if9zGjX=7f_c23asOFiKEEB7Q@mail.gmail.com>

Christian Couder <christian.couder@gmail.com> writes:

Show 6 quoted lines

> In the few cases where do_something() accepts NULL and does something
> different with it, the former can be changed to:
>
>         if (skip_to_optional_val(arg, "--key", &arg, NULL) /* the last
> argument is the default value */
>                 do_something(arg);

That's the thing. If do_something() that takes "" and NULL and behaves differently is rare, that indicates that the existing code may not be committed to treat "--key" and "--key=''" the same in the first place, and I am not 100% convinced that I want to see us committed to force that design throughout the system by introducing a helper that hardcodes the equivalence and encourages to use it.

Imagine a command that takes "--do-something" option and does that "something" unconditionally. We may later extend it to take an optional argument, i.e. "--do-something=c1,c2,...", to tell the command to do that "something" under some but not all conditions. The values c1,c2 would tell that we want that something done only under either conditions c1 or c2 holds true.

It would be natural to expect that "--do-something=" to do that "something" under no condition (i.e. as if no such option was given); that would help scripts that accumulate the set of conditions in a variable and say "--do-something=$when", by making it a no-op when the variable $when turns out to be an empty string. "--do-something" without "=" would not want to mean the same thing.

The above observation makes me suspect that it depends on the "key" what "--key=$value" we want to be equivalent to "--key". In the "--do-something" case, we do not want to pretend as if we got an empty string; instead we'd pretend as if we got "always" or something like that.

And your "default" would work well for this "default is tied to what the key is" paradigm, i.e.

	skip_to_optional_arg(arg, "--do-something", &arg, "always")

would make us treat "--do-something" and "--do-something=always" the same way.

If it turns out that the default arg almost always is an empty string, I do not mind

	#define skip_to_opt_arg(s,k,v) skip_to_optional_arg(s,k,v,"")

of course.