Re: [PATCH] C: use skip-prefix to avoid hardcoded string length

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

>> +		else if (skip_prefix(arg, "--expire=", &valptr)) {
>> +			if (parse_expiry_date(valptr, &cb.cmd.expire_total))
>>  				die(_("'%s' is not a valid timestamp"), arg);
>>  			explicit_expiry |= EXPIRE_TOTAL;
>>  		}
>
> In this case, I think the die message in the context could be improved
> to show "valptr". At which point you might as well do:
>
>   else if (skip_prefix(arg, "--expire=", &arg)) {

The version that loses "to which parameter did I give malformed
timestamp?" information was what I originally have written, and then
I added a new valptr variable to avoid the information loss and
message change.

But thinking about it again,

	git frotz --expire=tea --expire-unreachable=tee

would say "I don't know 'tee'" and then the user can go back and see
to which one the misspelt version went, and if the user did

	git frotz --expire=tee --expire-unreachable=tee

and got "I don't know 'tee'", then it also is OK to give that
without saying it is about --expire or --expire-unreachable; they
are both wrong ;-)

So, I guess it probably is a good idea to skip the option name in
the error message (we might have to adjust some tests, though).

Thanks.

>> -		else if (starts_with(arg, "--expire-unreachable=")) {
>> -			if (parse_expiry_date(arg + 21, &cb.cmd.expire_unreachable))
>> +		else if (skip_prefix(arg, "--expire-unreachable=", &valptr)) {
>> +			if (parse_expiry_date(valptr, &cb.cmd.expire_unreachable))
>>  				die(_("'%s' is not a valid timestamp"), arg);
>>  			explicit_expiry |= EXPIRE_UNREACH;
>>  		}
>
> Ditto here.
>
> -Peff