Re: [PATCHv5 7/7] Improve error handling when parsing dirstat parameters

[Johan Herland <johan@herland.net>]

On Thursday 28 April 2011, Junio C Hamano wrote:
> 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.

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

> 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