Git fork

parse-options: support unit factors in `OPT_INTEGER()`

There are two main differences between `OPT_INTEGER()` and
`OPT_MAGNITUDE()`:

- The former parses signed integers whereas the latter parses unsigned
integers.

- The latter parses unit factors like 'k', 'm' or 'g'.

While the first difference makes obvious sense, there isn't really a
good reason why signed integers shouldn't support unit factors, too.

This inconsistency will also become a bit of a problem with subsequent
commits, where we will fix a couple of callsites that pass an unsigned
integer to `OPT_INTEGER()`. There are three options:

- We could adapt those users to instead pass a signed integer, but
this would needlessly extend the range of accepted integer values.

- We could convert them to use `OPT_MAGNITUDE()`, as it only accepts
unsigned integers. But now we have the inconsistency that we also
start to accept unit factors.

- We could introduce `OPT_UNSIGNED()` as equivalent to `OPT_INTEGER()`
so that it knows to only accept unsigned integers without unit
suffix.

Introducing a whole new option type feels a bit excessive. There also
isn't really a good reason why `OPT_INTEGER()` cannot be extended to
also accept unit factors: all valid values passed to such options cannot
have a unit factors right now, so there wouldn't be any ambiguity.

Refactor `OPT_INTEGER()` to use `git_parse_int()`, which knows to
interpret unit factors. This removes the inconsistency between the
signed and unsigned options so that we can easily fix up callsites that
pass the wrong integer type right now.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

authored by

Patrick Steinhardt and committed by
Junio C Hamano
8ff1a34b d012ceb5

+11 -7
+4 -2
Documentation/technical/api-parse-options.adoc
··· 211 211 Use of `--no-option` will clear the list of preceding values. 212 212 213 213 `OPT_INTEGER(short, long, &int_var, description)`:: 214 - Introduce an option with integer argument. 215 - The integer is put into `int_var`. 214 + Introduce an option with integer argument. The argument must be a 215 + integer and may include a suffix of 'k', 'm' or 'g' to 216 + scale the provided value by 1024, 1024^2 or 1024^3 respectively. 217 + The scaled value is put into `int_var`. 216 218 217 219 `OPT_MAGNITUDE(short, long, &unsigned_long_var, description)`:: 218 220 Introduce an option with a size argument. The argument must be a
+4 -4
parse-options.c
··· 73 73 enum opt_parsed flags, 74 74 const char **argp) 75 75 { 76 - const char *s, *arg; 76 + const char *arg; 77 77 const int unset = flags & OPT_UNSET; 78 78 int err; 79 79 ··· 185 185 if (!*arg) 186 186 return error(_("%s expects a numerical value"), 187 187 optname(opt, flags)); 188 - *(int *)opt->value = strtol(arg, (char **)&s, 10); 189 - if (*s) 190 - return error(_("%s expects a numerical value"), 188 + if (!git_parse_int(arg, opt->value)) 189 + return error(_("%s expects an integer value" 190 + " with an optional k/m/g suffix"), 191 191 optname(opt, flags)); 192 192 return 0; 193 193
+3 -1
t/t0040-parse-options.sh
··· 111 111 112 112 test_expect_success 'OPT_BOOL() positivation' 'check boolean: 0 -D --doubt' 113 113 114 - test_expect_success 'OPT_INT() negative' 'check integer: -2345 -i -2345' 114 + test_expect_success 'OPT_INTEGER() negative' 'check integer: -2345 -i -2345' 115 + test_expect_success 'OPT_INTEGER() kilo' 'check integer: 239616 -i 234k' 116 + test_expect_success 'OPT_INTEGER() negative kilo' 'check integer: -239616 -i -234k' 115 117 116 118 test_expect_success 'OPT_MAGNITUDE() simple' ' 117 119 check magnitude: 2345678 -m 2345678