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

Ramkumar Ramachandra [off-list ref] writes:

Matthieu Moy wrote:

quoted

[...]

Before we begin, let me say that this series is written with the
emergency-fix mindset, and targets maint.

maint shouldn't be necessary since the breakage hasn't been released. At
worse, we can revert the bad commits now and re-implement them properly
later.

I didn't spend time thinking about how to do it best or even add
tests.

No problem.

quoted

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

I think you meant "what effect the config parser had". If you meant the
CLI parser, then the guilty commits did not change anything wrt that.

quoted

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

I don't think it's the first instance, but I can't remember precise
examples.

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?

That's an option too, yes. But probably not easy to implement :-(.

quoted

quoted

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

Should look like this:

diff --git a/t/t7508-status.sh b/t/t7508-status.sh
index 498332c..423e8c4 100755
--- a/t/t7508-status.sh
+++ b/t/t7508-status.sh

@@ -1378,6 +1378,11 @@ test_expect_success '"status.branch=true" weaker than "--no-branch"' '
        test_cmp expected_nobranch actual
 '
 
+test_expect_success '"status.branch=true" weaker than "--porcelain"' '
+       git -c status.branch=true status --porcelain >actual &&
+       test_cmp expected_nobranch actual
+'
+
 test_expect_success '"status.branch=false" same as "--no-branch"' '
        git -c status.branch=false status -s >actual &&
        test_cmp expected_nobranch actual

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