Re: [PATCHv5 7/7] Improve error handling when parsing dirstat parameters
- From
Johan Herland <johan@herland.net>
- Date
- Apr 28, 2011, 23:16 UTC
- Message-ID
- <201104290116.28647.johan@herland.net>
- In-Reply-To
- <7vd3k641zn.fsf@alter.siamese.dyndns.org>
On Thursday 28 April 2011, Junio C Hamano wrote:
Show 10 quoted lines
> Junio C Hamano <gitster@pobox.com> writes: > > I am not sure if three combinations (vanilla, -M and -C -C) need to be > > tested to produce an empty result. If so, it would make it easier to > > read if they are split into three tests, or at least have a blank line > > between them, but I suspect that you would agree that it is not worth > > to have three separate test_expect_success for these. > > I think it makes sense to cull these three cases into one for the case we > expect the command to stop without doing anything, but we would still > want to validate the output for three variants in the "config" case.
Agreed.
Show 8 quoted lines
> Also I forgot to say that the new "grep" invocations added to check the > error output might have to be test_i18ngrep. Please check with > > make GETTEXT_POISON=YesPlease test > > The configuration variable names and typo in user input should appear > somewhere in the output for any real locale, but I think gettext-poison > would throw these away.
Hmm. Not exactly sure how this is supposed to work. I ran the above command (after a test merge with 'pu' to get GETTEXT_POISON in my working tree), and it succeeded. But then, I have not marked my added strings for translation with "_()". Should I? AFAICS no other strings in diff.c are marked for translation either...
Show 6 quoted lines
> By the way, should the following two entries make any difference, and if > so how? > > [diff] > dirstat = unknown,0,lines > dirstat = 0,lines,unknown
No difference. The "rules" that apply in this case are: - Tokens are separated by commas - Unrecognized tokens are ignored
This is fundamentally what 7/7 tries to accomplish.
...Johan
-- Johan Herland, <johan@herland.net> www.herland.net