Matthieu Moy wrote:
> [...]

Before we begin, let me say that this series is written with the
emergency-fix mindset, and targets maint.  I didn't spend time
thinking about how to do it best or even add tests.

>> The correct solution is therefore to skip the config parser completely
>> when --porcelain is given.  Unfortunately, to determine that --porcelain
>> is given, we have to run the command-line option parser.  Running the
>> command-line option parser before the config parser is undesirable, as
>> configuration variables would override options on the command-line.  As
>> a fair compromise, check that argv[1] is equal to the string
>> "--porcelain" and skip the config parser in this case.
> I really don't like this. If we go for a solution looking explicitely at
> argv[], we should at least iterate over it (also not satisfactory
> because --porcelain could be the argument of another switch).

Yep, that's the compromise.

> I think it's possible to have an actually robust solution, either
> * running the CLI parser after, if --porcelain is given, reset the
>   effect of the variables. Not very clean because we'd have to reset all
>   the variables to their default, and there is a risk of forgetting one.

Since it's impossible to determine what effect the CLI parser had on
various variables (some of which are static global), I'm against this

> * Or, running the CLI parser before, but with different variables to
>   specify what the command-line says and what will actually be done,
>   with something like

Basically, having the CLI parser and the config parser flip two
different sets of variables, so we can discriminate who set what.
What annoys me is that this is the first instance of such a

The approach I'm currently tilting towards is extending the
parse-options API to allow parsing one special option early.  I would
argue that this is a good feature that we should have asked for when
we saw 6758af89e (Merge branch 'jn/git-cmd-h-bypass-setup',
2010-12-10).  What do you think?

>>  builtin/commit.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
> No time to contribute one now myself, but this would really deserve a
> test.

Yeah, will do as a follow-up.  I'm interested in guarding against such
breakages too :)
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to
More majordomo info at

Reply via email to