parse-options API: remove OPTION_ARGUMENT feature

As was noted in 1a85b49b87a (parse-options: make OPT_ARGUMENT() more useful, 2019-03-14) there's only ever been one user of the OPT_ARGUMENT(), that user was added in 20de316e334 (difftool: allow running outside Git worktrees with --no-index, 2019-03-14). The OPT_ARGUMENT() feature itself was added way back in 580d5bffdea (parse-options: new option type to treat an option-like parameter as an argument., 2008-03-02), but as discussed in 1a85b49b87a wasn't used until 20de316e334 in 2019. Now that the preceding commit has migrated this code over to using "struct strvec" to manage the "args" member of a "struct child_process", we can just use that directly instead of relying on OPT_ARGUMENT. This has a minor change in behavior in that if we'll pass --no-index we'll now always pass it as the first argument, before we'd pass it in whatever position the caller did. Preserving this was the real value of OPT_ARGUMENT(), but as it turns out we didn't need that either. We can always inject it as the first argument, the other end will parse it just the same. Note that we cannot remove the "out" and "cpidx" members of "struct parse_opt_ctx_t" added in 580d5bffdea, while they were introduced with OPT_ARGUMENT() we since used them for other things. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Sep 13, 2021 at 05:35 UTC 4c25356e0edaa92e21aadb67579a0263019fbdc4
6 files changed +3 -28
Documentation/technical/api-parse-options.txt
-5
@@ -198,11 +198,6 @@ There are some macros to easily define options:
198 The filename will be prefixed by passing the filename along with
199 the prefix argument of `parse_options()` to `prefix_filename()`.
200
201 -`OPT_ARGUMENT(long, &int_var, description)`::
202 - Introduce a long-option argument that will be kept in `argv[]`.
203 - If this option was seen, `int_var` will be set to one (except
204 - if a `NULL` pointer was passed).
205 -
201 `OPT_NUMBER_CALLBACK(&var, description, func_ptr)`::
202 Recognize numerical options like -123 and feed the integer as
203 if it was an argument to the function given by `func_ptr`.
builtin/difftool.c
+3 -1
@@ -709,7 +709,7 @@ int cmd_difftool(int argc, const char **argv, const char *prefix)
709 "tool returns a non - zero exit code")),
710 OPT_STRING('x', "extcmd", &extcmd, N_("command"),
711 N_("specify a custom command for viewing diffs")),
712 - OPT_ARGUMENT("no-index", &no_index, N_("passed to `diff`")),
712 + OPT_BOOL(0, "no-index", &no_index, N_("passed to `diff`")),
713 OPT_END()
714 };
715 struct child_process child = CHILD_PROCESS_INIT;
@@ -763,6 +763,8 @@ int cmd_difftool(int argc, const char **argv, const char *prefix)
763 * each file that changed.
764 */
765 strvec_push(&child.args, "diff");
766 + if (no_index)
767 + strvec_push(&child.args, "--no-index");
768 if (dir_diff)
769 strvec_pushl(&child.args, "--raw", "--no-abbrev", "-z", NULL);
770 strvec_pushv(&child.args, argv);
parse-options.c
-13
@@ -310,19 +310,6 @@ static enum parse_opt_result parse_long_opt(
310 again:
311 if (!skip_prefix(arg, long_name, &rest))
312 rest = NULL;
313 - if (options->type == OPTION_ARGUMENT) {
314 - if (!rest)
315 - continue;
316 - if (*rest == '=')
317 - return error(_("%s takes no value"),
318 - optname(options, flags));
319 - if (*rest)
320 - continue;
321 - if (options->value)
322 - *(int *)options->value = options->defval;
323 - p->out[p->cpidx++] = arg - 2;
324 - return PARSE_OPT_DONE;
325 - }
313 if (!rest) {
314 /* abbreviated? */
315 if (!(p->flags & PARSE_OPT_KEEP_UNKNOWN) &&
parse-options.h
-3
@@ -8,7 +8,6 @@
8 enum parse_opt_type {
9 /* special types */
10 OPTION_END,
11 - OPTION_ARGUMENT,
11 OPTION_GROUP,
12 OPTION_NUMBER,
13 OPTION_ALIAS,
@@ -155,8 +154,6 @@ struct option {
154 #define OPT_INTEGER_F(s, l, v, h, f) { OPTION_INTEGER, (s), (l), (v), N_("n"), (h), (f) }
155
156 #define OPT_END() { OPTION_END }
158 -#define OPT_ARGUMENT(l, v, h) { OPTION_ARGUMENT, 0, (l), (v), NULL, \
159 - (h), PARSE_OPT_NOARG, NULL, 1 }
157 #define OPT_GROUP(h) { OPTION_GROUP, 0, NULL, NULL, NULL, (h) }
158 #define OPT_BIT(s, l, v, h, b) OPT_BIT_F(s, l, v, h, b, 0)
159 #define OPT_BITOP(s, l, v, h, set, clear) { OPTION_BITOP, (s), (l), (v), NULL, (h), \
t/helper/test-parse-options.c
-1
@@ -134,7 +134,6 @@ int cmd__parse_options(int argc, const char **argv)
134 OPT_NOOP_NOARG(0, "obsolete"),
135 OPT_STRING_LIST(0, "list", &list, "str", "add str to list"),
136 OPT_GROUP("Magic arguments"),
137 - OPT_ARGUMENT("quux", NULL, "means --quux"),
137 OPT_NUMBER_CALLBACK(&integer, "set integer to NUM",
138 number_callback),
139 { OPTION_COUNTUP, '+', NULL, &boolean, NULL, "same as -b",
t/t0040-parse-options.sh
-5
@@ -37,7 +37,6 @@ String options
37 --list <str> add str to list
38
39 Magic arguments
40 - --quux means --quux
40 -NUM set integer to NUM
41 + same as -b
42 --ambiguous positive ambiguity
@@ -263,10 +262,6 @@ test_expect_success 'detect possible typos' '
262 test_cmp typo.err output.err
263 '
264
266 -test_expect_success 'keep some options as arguments' '
267 - test-tool parse-options --expect="arg 00: --quux" --quux
268 -'
269 -
265 cat >expect <<\EOF
266 Callback: "four", 0
267 boolean: 5