Re: [PATCH 1/2] status: really ignore config with --porcelain

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Jun 24, 2013, 13:51 UTC
Message-ID: <vpqhagnwraj.fsf@anie.imag.fr>
In-Reply-To: <1372077912-18625-2-git-send-email-artagnon@gmail.com>

Ramkumar Ramachandra <artagnon@gmail.com> writes:

Show 6 quoted lines

> Contrary to its description, the output of
>
>   $ git status --porcelain
>
> now depends on the configuration variables status.short and
> status.branch.

Ouch, indeed :-(.

Show 7 quoted lines

> 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).

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.

* 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

  actual_short = 0;
  switch (command_line_short) {
  case yes:
          actual_short = 1;
          break;
  case no:
          actual_short = 0;
          break;
  case unset: /* nothing */
  }
  switch (config_short) {
  // same
  }

> ---
>  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.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/