git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Johannes Schindelin" <Johannes.Schindelin@gmx.de>,
	"Jeff King" <peff@peff.net>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Subject: [PATCH v2 4/4] parse-options API: remove OPTION_ARGUMENT feature
Date: Mon, 13 Sep 2021 05:35:40 +0200	[thread overview]
Message-ID: <patch-v2-4.4-4fddce0a38d-20210913T033204Z-avarab@gmail.com> (raw)
In-Reply-To: <cover-v2-0.4-00000000000-20210913T033204Z-avarab@gmail.com>

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>
---
 Documentation/technical/api-parse-options.txt |  5 -----
 builtin/difftool.c                            |  4 +++-
 parse-options.c                               | 13 -------------
 parse-options.h                               |  3 ---
 t/helper/test-parse-options.c                 |  1 -
 t/t0040-parse-options.sh                      |  5 -----
 6 files changed, 3 insertions(+), 28 deletions(-)

diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt
index 5a60bbfa7f4..acfd5dc1d8b 100644
--- a/Documentation/technical/api-parse-options.txt
+++ b/Documentation/technical/api-parse-options.txt
@@ -198,11 +198,6 @@ There are some macros to easily define options:
 	The filename will be prefixed by passing the filename along with
 	the prefix argument of `parse_options()` to `prefix_filename()`.
 
-`OPT_ARGUMENT(long, &int_var, description)`::
-	Introduce a long-option argument that will be kept in `argv[]`.
-	If this option was seen, `int_var` will be set to one (except
-	if a `NULL` pointer was passed).
-
 `OPT_NUMBER_CALLBACK(&var, description, func_ptr)`::
 	Recognize numerical options like -123 and feed the integer as
 	if it was an argument to the function given by `func_ptr`.
diff --git a/builtin/difftool.c b/builtin/difftool.c
index de2e5545c81..bb9fe7245a4 100644
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@ -709,7 +709,7 @@ int cmd_difftool(int argc, const char **argv, const char *prefix)
 			    "tool returns a non - zero exit code")),
 		OPT_STRING('x', "extcmd", &extcmd, N_("command"),
 			   N_("specify a custom command for viewing diffs")),
-		OPT_ARGUMENT("no-index", &no_index, N_("passed to `diff`")),
+		OPT_BOOL(0, "no-index", &no_index, N_("passed to `diff`")),
 		OPT_END()
 	};
 	struct child_process child = CHILD_PROCESS_INIT;
@@ -763,6 +763,8 @@ int cmd_difftool(int argc, const char **argv, const char *prefix)
 	 * each file that changed.
 	 */
 	strvec_push(&child.args, "diff");
+	if (no_index)
+		strvec_push(&child.args, "--no-index");
 	if (dir_diff)
 		strvec_pushl(&child.args, "--raw", "--no-abbrev", "-z", NULL);
 	strvec_pushv(&child.args, argv);
diff --git a/parse-options.c b/parse-options.c
index 2abff136a17..55c5821b08d 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -310,19 +310,6 @@ static enum parse_opt_result parse_long_opt(
 again:
 		if (!skip_prefix(arg, long_name, &rest))
 			rest = NULL;
-		if (options->type == OPTION_ARGUMENT) {
-			if (!rest)
-				continue;
-			if (*rest == '=')
-				return error(_("%s takes no value"),
-					     optname(options, flags));
-			if (*rest)
-				continue;
-			if (options->value)
-				*(int *)options->value = options->defval;
-			p->out[p->cpidx++] = arg - 2;
-			return PARSE_OPT_DONE;
-		}
 		if (!rest) {
 			/* abbreviated? */
 			if (!(p->flags & PARSE_OPT_KEEP_UNKNOWN) &&
diff --git a/parse-options.h b/parse-options.h
index a845a9d9527..39d90882548 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -8,7 +8,6 @@
 enum parse_opt_type {
 	/* special types */
 	OPTION_END,
-	OPTION_ARGUMENT,
 	OPTION_GROUP,
 	OPTION_NUMBER,
 	OPTION_ALIAS,
@@ -155,8 +154,6 @@ struct option {
 #define OPT_INTEGER_F(s, l, v, h, f)     { OPTION_INTEGER, (s), (l), (v), N_("n"), (h), (f) }
 
 #define OPT_END()                   { OPTION_END }
-#define OPT_ARGUMENT(l, v, h)       { OPTION_ARGUMENT, 0, (l), (v), NULL, \
-				      (h), PARSE_OPT_NOARG, NULL, 1 }
 #define OPT_GROUP(h)                { OPTION_GROUP, 0, NULL, NULL, NULL, (h) }
 #define OPT_BIT(s, l, v, h, b)      OPT_BIT_F(s, l, v, h, b, 0)
 #define OPT_BITOP(s, l, v, h, set, clear) { OPTION_BITOP, (s), (l), (v), NULL, (h), \
diff --git a/t/helper/test-parse-options.c b/t/helper/test-parse-options.c
index 2051ce57db7..a282b6ff13e 100644
--- a/t/helper/test-parse-options.c
+++ b/t/helper/test-parse-options.c
@@ -134,7 +134,6 @@ int cmd__parse_options(int argc, const char **argv)
 		OPT_NOOP_NOARG(0, "obsolete"),
 		OPT_STRING_LIST(0, "list", &list, "str", "add str to list"),
 		OPT_GROUP("Magic arguments"),
-		OPT_ARGUMENT("quux", NULL, "means --quux"),
 		OPT_NUMBER_CALLBACK(&integer, "set integer to NUM",
 			number_callback),
 		{ OPTION_COUNTUP, '+', NULL, &boolean, NULL, "same as -b",
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index ad4746d899a..da310ed29b1 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -37,7 +37,6 @@ String options
     --list <str>          add str to list
 
 Magic arguments
-    --quux                means --quux
     -NUM                  set integer to NUM
     +                     same as -b
     --ambiguous           positive ambiguity
@@ -263,10 +262,6 @@ test_expect_success 'detect possible typos' '
 	test_cmp typo.err output.err
 '
 
-test_expect_success 'keep some options as arguments' '
-	test-tool parse-options --expect="arg 00: --quux" --quux
-'
-
 cat >expect <<\EOF
 Callback: "four", 0
 boolean: 5
-- 
2.33.0.999.ga5f89b684e9


  parent reply	other threads:[~2021-09-13  3:45 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-11 18:21 [PATCH 0/2] parse-options.c: remove OPT_ARGUMENT Ævar Arnfjörð Bjarmason
2021-09-11 18:21 ` [PATCH 1/2] difftool: use "struct strvec" API in run_{dir,file}_diff() Ævar Arnfjörð Bjarmason
2021-09-12 22:39   ` Jeff King
2021-09-12 22:41     ` Jeff King
2021-09-13  0:10     ` Junio C Hamano
2021-09-11 18:21 ` [PATCH 2/2] parse-options API: remove OPTION_ARGUMENT feature Ævar Arnfjörð Bjarmason
2021-09-12 22:43   ` Jeff King
2021-09-13  3:35 ` [PATCH v2 0/4] difftool refactoring + remove OPT_ARGUMENT() macro Ævar Arnfjörð Bjarmason
2021-09-13  3:35   ` [PATCH v2 1/4] difftool: prepare "struct child_process" in cmd_difftool() Ævar Arnfjörð Bjarmason
2021-09-13  3:35   ` [PATCH v2 2/4] difftool: prepare "diff" cmdline " Ævar Arnfjörð Bjarmason
2021-09-13  3:35   ` [PATCH v2 3/4] difftool: use run_command() API in run_file_diff() Ævar Arnfjörð Bjarmason
2021-09-13 18:04     ` Jeff King
2021-09-13 19:13       ` Ævar Arnfjörð Bjarmason
2021-09-13 19:27         ` Jeff King
2021-09-13  3:35   ` Ævar Arnfjörð Bjarmason [this message]
2021-09-13  6:27     ` [PATCH v2 4/4] parse-options API: remove OPTION_ARGUMENT feature Junio C Hamano
2021-09-13 18:04   ` [PATCH v2 0/4] difftool refactoring + remove OPT_ARGUMENT() macro Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=patch-v2-4.4-4fddce0a38d-20210913T033204Z-avarab@gmail.com \
    --to=avarab@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).