Re: [PATCH 1/2] status: really ignore config with --porcelain
- From
Matthieu Moy <matthieu.moy@grenoble-inp.fr>
- Date
- Jun 24, 2013, 14:51 UTC
- Message-ID
- <vpqhagnv9xq.fsf@anie.imag.fr>
- In-Reply-To
- <CALkWK0=F_i95S+53eZmOAJtA+jG=jvi5-sDc3BgW3rNQo=n3Ng@mail.gmail.com>
Ramkumar Ramachandra <artagnon@gmail.com> writes:
Show 5 quoted lines
> Matthieu Moy wrote: >> [...] > > 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.
Show 7 quoted lines
>> * 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.
Show 9 quoted lines
> >> * 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.
Show 5 quoted lines
> 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 :-(.
Show 8 quoted lines
>>> 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/