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

Jeff King [off-list ref] writes:

quoted

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

quoted

-		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