Charles Bailey <charles@xxxxxxxxxxxxx> writes:
> diff --git a/parse-options.h b/parse-options.h
> index c71e9da..2ddb26f 100644
> --- a/parse-options.h
> +++ b/parse-options.h
> @@ -18,7 +18,8 @@ enum parse_opt_type {
> OPTION_INTEGER,
> OPTION_CALLBACK,
> OPTION_LOWLEVEL_CALLBACK,
> - OPTION_FILENAME
> + OPTION_FILENAME,
> + OPTION_ULONG
> };
Please place it immediately after INTEGER, as they are conceptually
siblings---group similar things together.
> return opterror(opt, "expects a numerical value", flags);
> return 0;
>
> + case OPTION_ULONG:
This one is placed right from that point of view ;-)
> + if (unset) {
> + *(unsigned long *)opt->value = 0;
> + return 0;
> + }
> + if (opt->flags & PARSE_OPT_OPTARG && !p->opt) {
> + *(unsigned long *)opt->value = opt->defval;
> + return 0;
> + }
> + if (get_arg(p, opt, flags, &arg))
> + return -1;
> + if (!git_parse_ulong(arg, opt->value))
> + return opterror(opt, "expects a numerical value", flags);
This used to be:
> - die(_("unable to parse value '%s' for option %s"),
> - arg, opt->long_name);
but opterror() talks about which option, so there is no information
loss by losing "for option %s" from here. That means there is only
one difference for pack-objects:
$ git pack-objects --max-pack-size=1T
fatal: unable to parse value '1T' for option max-pack-size
$ ./git pack-objects --max-pack-size=1T
error: option `max-pack-size' expects a numerical value
usage: git pack-objects --stdout [options...
... 30 more lines omitted ...
Eh, make that two:
* We no longer say what value we did not like. The user presumably
knows what he typed, so this is only a minor loss.
* We used to stop without giving "usage", as the error message was
specific enough. We now spew descriptions on other options
unrelated to the specific error the user may want to concentrate
on. Perhaps this is a minor regression.
I wonder if "expects a numerical value" is the best way to say this.
Ponder:
- we do not take "4.8"
- we do not take "-4".
- people may not realize, from "numerical", that we take "5M".
Except for the minor nits above, I think this is a good change.
This is a totally unrelated tangent that does not have to be part of
your series, but we probably should take "4.8M"; I do not think we
currently do.
Oh, and perhaps 1T, too.
--
To unsubscribe from this list: send the line "unsubscribe git" in