Jonathan Nieder <jrnieder@xxxxxxxxx> writes:
> (a) rename has_dash_dash here to dash_dash_pos, or
> (b) put the check in the loop, like so:
I agree with (a), but not with (b). I think separating the computation
of the position and the diagnosis makes it clearer.
I reworked the code a bit, the diagnosis part now looks like
if (dash_dash_pos == 0)
return 1; /* case (2) */
else if (dash_dash_pos == 1)
has_dash_dash = 1; /* case (3) or (1) */
else if (dash_dash_pos >= 2)
die(_("only one reference expected, %d given."), dash_dash_pos);
>> +test_expect_success C_LOCALE_OUTPUT 'accurate error message with more than one ref' '
>> + test_must_fail git checkout HEAD master -- 2>actual &&
>> + echo "fatal: only one reference expected, 2 given." >expect &&
>> + test_cmp expect actual
>
> Nits:
>
> - if we change this from 'fatal' to 'error' or reword the message as
> part of a libification some day, it would be a nuisance to have to
> update this test. Maybe a 'grep' could make it more flexible.
>
> - most of the test (though not the interesting part) can run in the
> !C_LOCALE_OUTPUT case if you use test_i18ncmp or test_i18ngrep
>
> - even the check for "2" can run in the !C_LOCALE_OUTPUT case. :)
> e.g. something like
>
> test_must_fail ... 2>actual &&
> grep 2 actual &&
> test_i18ngrep "one reference expected, 2 given" actual
OK, I buy your version. Thanks,
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html