Re: [PATCH v2 2/2] checkout: proper error message on 'git checkout foo bar --'

Jonathan Nieder [off-list ref] 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);

quoted

+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/