Re: [PATCH v3 3/3] send-email: do defaults -> config -> getopt in that order

From: Junio C Hamano <gitster@pobox.com>
Date: May 16, 2019, 23:13 UTC
Message-ID: <xmqqh89u3sdw.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <5cddeb61.1c69fb81.47ed4.e648@mx.google.com>

Stephen Boyd <swboyd@chromium.org> writes:

Show 20 quoted lines

>> As can be seen in this change the only place where we actually want to
>> do something clever is with the to/cc/bcc variables, where setting
>> them on the command-line (or using --no-{to,cc,bcc}) should clear out
>> values we grab from the config.
>> 
>> All the rest are things where the command-line should simply override
>> the config values, and by reading the config first the config code
>> doesn't need all this "let's not set it was on the command-line"
>> special-casing, as [1] shows we'd otherwise need to care about the
>> difference between whether something was a default or present in
>> config to fix the bug in e67a228cd8.
>
> This broke my workflow.
>
> I specify --identity=<account> on the commandline and I want that to
> pick out my send-email config from my global .gitconfig file that
> corresponds to that identity. With this change, the config is parsed
> before the getopt part so --identity on the commandline is a nop and
> never looks into the config file to figure this out. So at least
> --identity is special in addition to --to,cc,bcc.

Ah, sorry that nobody noticed that case, but you are right. Because the ident is used as a part of the key to find identity-specific configuration values, if the command line gives one, we must have an access to it before we start reading the configuration. In that sense, it is more fundamental to special-case the option.

We are past -rc0, so I am inclined to revert the change (and perhaps replace it with the other "fix" that did not break the parsing order like these patches did), with an expectation that a clever fix will be found later, *unless* a simple and correct fix is found quickly.