parse-options: move unsigned long option parsing out of pack-objects.c

The unsigned long option parsing (including 'k'/'m'/'g' suffix parsing) is more widely applicable. Add support for OPT_MAGNITUDE to parse-options.h and change pack-objects.c use this support. The error behavior on parse errors follows that of OPT_INTEGER. The name of the option that failed to parse is reported with a brief message describing the expect format for the option argument and then the full usage message for the command invoked. This differs from the previous behavior for OPT_ULONG used in pack-objects for --max-pack-size and --window-memory which used to display the value supplied in the error message and did not display the full usage message. Signed-off-by: Charles Bailey <cbailey32@bloomberg.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Charles Bailey committed Jun 21, 2015 at 19:25 UTC 2a514ed8058e35841d3d7b05a898991b83e5eaf0
6 files changed +73 -26
Documentation/technical/api-parse-options.txt
+6
@@ -168,6 +168,12 @@ There are some macros to easily define options:
168 Introduce an option with integer argument.
169 The integer is put into `int_var`.
170
171 +`OPT_MAGNITUDE(short, long, &unsigned_long_var, description)`::
172 + Introduce an option with a size argument. The argument must be a
173 + non-negative integer and may include a suffix of 'k', 'm' or 'g' to
174 + scale the provided value by 1024, 1024^2 or 1024^3 respectively.
175 + The scaled value is put into `unsigned_long_var`.
176 +
177 `OPT_DATE(short, long, &int_var, description)`::
178 Introduce an option with date argument, see `approxidate()`.
179 The timestamp is put into `int_var`.
builtin/pack-objects.c
+4 -21
@@ -2588,23 +2588,6 @@ static int option_parse_unpack_unreachable(const struct option *opt,
2588 return 0;
2589 }
2590
2591 -static int option_parse_ulong(const struct option *opt,
2592 - const char *arg, int unset)
2593 -{
2594 - if (unset)
2595 - die(_("option %s does not accept negative form"),
2596 - opt->long_name);
2597 -
2598 - if (!git_parse_ulong(arg, opt->value))
2599 - die(_("unable to parse value '%s' for option %s"),
2600 - arg, opt->long_name);
2601 - return 0;
2602 -}
2603 -
2604 -#define OPT_ULONG(s, l, v, h) \
2605 - { OPTION_CALLBACK, (s), (l), (v), "n", (h), \
2606 - PARSE_OPT_NONEG, option_parse_ulong }
2607 -
2591 int cmd_pack_objects(int argc, const char **argv, const char *prefix)
2592 {
2593 int use_internal_rev_list = 0;
@@ -2627,16 +2610,16 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
2610 { OPTION_CALLBACK, 0, "index-version", NULL, N_("version[,offset]"),
2611 N_("write the pack index file in the specified idx format version"),
2612 0, option_parse_index_version },
2630 - OPT_ULONG(0, "max-pack-size", &pack_size_limit,
2631 - N_("maximum size of each output pack file")),
2613 + OPT_MAGNITUDE(0, "max-pack-size", &pack_size_limit,
2614 + N_("maximum size of each output pack file")),
2615 OPT_BOOL(0, "local", &local,
2616 N_("ignore borrowed objects from alternate object store")),
2617 OPT_BOOL(0, "incremental", &incremental,
2618 N_("ignore packed objects")),
2619 OPT_INTEGER(0, "window", &window,
2620 N_("limit pack window by objects")),
2638 - OPT_ULONG(0, "window-memory", &window_memory_limit,
2639 - N_("limit pack window by memory in addition to object limit")),
2621 + OPT_MAGNITUDE(0, "window-memory", &window_memory_limit,
2622 + N_("limit pack window by memory in addition to object limit")),
2623 OPT_INTEGER(0, "depth", &depth,
2624 N_("maximum length of delta chain allowed in the resulting pack")),
2625 OPT_BOOL(0, "reuse-delta", &reuse_delta,
parse-options.c
+17
@@ -180,6 +180,23 @@ static int get_value(struct parse_opt_ctx_t *p,
180 return opterror(opt, "expects a numerical value", flags);
181 return 0;
182
183 + case OPTION_MAGNITUDE:
184 + if (unset) {
185 + *(unsigned long *)opt->value = 0;
186 + return 0;
187 + }
188 + if (opt->flags & PARSE_OPT_OPTARG && !p->opt) {
189 + *(unsigned long *)opt->value = opt->defval;
190 + return 0;
191 + }
192 + if (get_arg(p, opt, flags, &arg))
193 + return -1;
194 + if (!git_parse_ulong(arg, opt->value))
195 + return opterror(opt,
196 + "expects a non-negative integer value with an optional k/m/g suffix",
197 + flags);
198 + return 0;
199 +
200 default:
201 die("should not happen, someone must be hit on the forehead");
202 }
parse-options.h
+3
@@ -16,6 +16,7 @@ enum parse_opt_type {
16 /* options with arguments (usually) */
17 OPTION_STRING,
18 OPTION_INTEGER,
19 + OPTION_MAGNITUDE,
20 OPTION_CALLBACK,
21 OPTION_LOWLEVEL_CALLBACK,
22 OPTION_FILENAME
@@ -129,6 +130,8 @@ struct option {
130 #define OPT_CMDMODE(s, l, v, h, i) { OPTION_CMDMODE, (s), (l), (v), NULL, \
131 (h), PARSE_OPT_NOARG|PARSE_OPT_NONEG, NULL, (i) }
132 #define OPT_INTEGER(s, l, v, h) { OPTION_INTEGER, (s), (l), (v), N_("n"), (h) }
133 +#define OPT_MAGNITUDE(s, l, v, h) { OPTION_MAGNITUDE, (s), (l), (v), \
134 + N_("n"), (h), PARSE_OPT_NONEG }
135 #define OPT_STRING(s, l, v, a, h) { OPTION_STRING, (s), (l), (v), (a), (h) }
136 #define OPT_STRING_LIST(s, l, v, a, h) \
137 { OPTION_CALLBACK, (s), (l), (v), (a), \
t/t0040-parse-options.sh
+40 -5
@@ -19,6 +19,7 @@ usage: test-parse-options <options>
19
20 -i, --integer <n> get a integer
21 -j <n> get a integer, too
22 + -m, --magnitude <n> get a magnitude
23 --set23 set integer to 23
24 -t <time> get timestamp of <time>
25 -L, --length <str> get length of <str>
@@ -58,6 +59,7 @@ mv expect expect.err
59 cat >expect.template <<EOF
60 boolean: 0
61 integer: 0
62 +magnitude: 0
63 timestamp: 0
64 string: (not set)
65 abbrev: 7
@@ -134,9 +136,30 @@ test_expect_success 'OPT_BOOL() positivation' 'check boolean: 0 -D --doubt'
136
137 test_expect_success 'OPT_INT() negative' 'check integer: -2345 -i -2345'
138
139 +test_expect_success 'OPT_MAGNITUDE() simple' '
140 + check magnitude: 2345678 -m 2345678
141 +'
142 +
143 +test_expect_success 'OPT_MAGNITUDE() kilo' '
144 + check magnitude: 239616 -m 234k
145 +'
146 +
147 +test_expect_success 'OPT_MAGNITUDE() mega' '
148 + check magnitude: 104857600 -m 100m
149 +'
150 +
151 +test_expect_success 'OPT_MAGNITUDE() giga' '
152 + check magnitude: 1073741824 -m 1g
153 +'
154 +
155 +test_expect_success 'OPT_MAGNITUDE() 3giga' '
156 + check magnitude: 3221225472 -m 3g
157 +'
158 +
159 cat > expect << EOF
160 boolean: 2
161 integer: 1729
162 +magnitude: 16384
163 timestamp: 0
164 string: 123
165 abbrev: 7
@@ -147,8 +170,8 @@ file: prefix/my.file
170 EOF
171
172 test_expect_success 'short options' '
150 - test-parse-options -s123 -b -i 1729 -b -vv -n -F my.file \
151 - > output 2> output.err &&
173 + test-parse-options -s123 -b -i 1729 -m 16k -b -vv -n -F my.file \
174 + >output 2>output.err &&
175 test_cmp expect output &&
176 test_must_be_empty output.err
177 '
@@ -156,6 +179,7 @@ test_expect_success 'short options' '
179 cat > expect << EOF
180 boolean: 2
181 integer: 1729
182 +magnitude: 16384
183 timestamp: 0
184 string: 321
185 abbrev: 10
@@ -166,9 +190,10 @@ file: prefix/fi.le
190 EOF
191
192 test_expect_success 'long options' '
169 - test-parse-options --boolean --integer 1729 --boolean --string2=321 \
170 - --verbose --verbose --no-dry-run --abbrev=10 --file fi.le\
171 - --obsolete > output 2> output.err &&
193 + test-parse-options --boolean --integer 1729 --magnitude 16k \
194 + --boolean --string2=321 --verbose --verbose --no-dry-run \
195 + --abbrev=10 --file fi.le --obsolete \
196 + >output 2>output.err &&
197 test_must_be_empty output.err &&
198 test_cmp expect output
199 '
@@ -182,6 +207,7 @@ test_expect_success 'missing required value' '
207 cat > expect << EOF
208 boolean: 1
209 integer: 13
210 +magnitude: 0
211 timestamp: 0
212 string: 123
213 abbrev: 7
@@ -204,6 +230,7 @@ test_expect_success 'intermingled arguments' '
230 cat > expect << EOF
231 boolean: 0
232 integer: 2
233 +magnitude: 0
234 timestamp: 0
235 string: (not set)
236 abbrev: 7
@@ -232,6 +259,7 @@ test_expect_success 'ambiguously abbreviated option' '
259 cat > expect << EOF
260 boolean: 0
261 integer: 0
262 +magnitude: 0
263 timestamp: 0
264 string: 123
265 abbrev: 7
@@ -270,6 +298,7 @@ test_expect_success 'detect possible typos' '
298 cat > expect <<EOF
299 boolean: 0
300 integer: 0
301 +magnitude: 0
302 timestamp: 0
303 string: (not set)
304 abbrev: 7
@@ -289,6 +318,7 @@ test_expect_success 'keep some options as arguments' '
318 cat > expect <<EOF
319 boolean: 0
320 integer: 0
321 +magnitude: 0
322 timestamp: 1
323 string: (not set)
324 abbrev: 7
@@ -310,6 +340,7 @@ cat > expect <<EOF
340 Callback: "four", 0
341 boolean: 5
342 integer: 4
343 +magnitude: 0
344 timestamp: 0
345 string: (not set)
346 abbrev: 7
@@ -338,6 +369,7 @@ test_expect_success 'OPT_CALLBACK() and callback errors work' '
369 cat > expect <<EOF
370 boolean: 1
371 integer: 23
372 +magnitude: 0
373 timestamp: 0
374 string: (not set)
375 abbrev: 7
@@ -362,6 +394,7 @@ test_expect_success 'OPT_NEGBIT() and OPT_SET_INT() work' '
394 cat > expect <<EOF
395 boolean: 6
396 integer: 0
397 +magnitude: 0
398 timestamp: 0
399 string: (not set)
400 abbrev: 7
@@ -392,6 +425,7 @@ test_expect_success 'OPT_COUNTUP() with PARSE_OPT_NODASH works' '
425 cat > expect <<EOF
426 boolean: 0
427 integer: 12345
428 +magnitude: 0
429 timestamp: 0
430 string: (not set)
431 abbrev: 7
@@ -410,6 +444,7 @@ test_expect_success 'OPT_NUMBER_CALLBACK() works' '
444 cat >expect <<EOF
445 boolean: 0
446 integer: 0
447 +magnitude: 0
448 timestamp: 0
449 string: (not set)
450 abbrev: 7
test-parse-options.c
+3
@@ -4,6 +4,7 @@
4
5 static int boolean = 0;
6 static int integer = 0;
7 +static unsigned long magnitude = 0;
8 static unsigned long timestamp;
9 static int abbrev = 7;
10 static int verbose = 0, dry_run = 0, quiet = 0;
@@ -48,6 +49,7 @@ int main(int argc, char **argv)
49 OPT_GROUP(""),
50 OPT_INTEGER('i', "integer", &integer, "get a integer"),
51 OPT_INTEGER('j', NULL, &integer, "get a integer, too"),
52 + OPT_MAGNITUDE('m', "magnitude", &magnitude, "get a magnitude"),
53 OPT_SET_INT(0, "set23", &integer, "set integer to 23", 23),
54 OPT_DATE('t', NULL, &timestamp, "get timestamp of <time>"),
55 OPT_CALLBACK('L', "length", &integer, "str",
@@ -83,6 +85,7 @@ int main(int argc, char **argv)
85
86 printf("boolean: %d\n", boolean);
87 printf("integer: %d\n", integer);
88 + printf("magnitude: %lu\n", magnitude);
89 printf("timestamp: %lu\n", timestamp);
90 printf("string: %s\n", string ? string : "(not set)");
91 printf("abbrev: %d\n", abbrev);