From ec3cc27ab06c0b7ebaf9aee3eaa6e35719eef8da Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?=
<avarab@gmail.com>
Date: Mon, 13 Sep 2021 05:35:37 +0200
Subject: [PATCH 1/4] difftool: prepare "struct child_process" in
cmd_difftool()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Move the preparation of the "struct child_process" from run_dir_diff()
to its only caller, cmd_difftool(). This is in preparation for
migrating run_file_diff() to using the run_command() API directly, and
to move more of the shared setup of the two to cmd_difftool().
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/difftool.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/builtin/difftool.c b/builtin/difftool.c
index 6a9242a803..9f08a8f3fd 100644
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@ -331,7 +331,8 @@ static int checkout_path(unsigned mode, struct object_id *oid,
}
static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
- int argc, const char **argv)
+ int argc, const char **argv,
+ struct child_process *child)
{
char tmpdir[PATH_MAX];
struct strbuf info = STRBUF_INIT, lpath = STRBUF_INIT;
@@ -352,7 +353,6 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
struct index_state wtindex;
struct checkout lstate, rstate;
int rc, flags = RUN_GIT_CMD, err = 0;
- struct child_process child = CHILD_PROCESS_INIT;
const char *helper_argv[] = { "difftool--helper", NULL, NULL, NULL };
struct hashmap wt_modified, tmp_modified;
int indices_loaded = 0;
@@ -387,19 +387,19 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
rdir_len = rdir.len;
wtdir_len = wtdir.len;
- child.no_stdin = 1;
- child.git_cmd = 1;
- child.use_shell = 0;
- child.clean_on_exit = 1;
- child.dir = prefix;
- child.out = -1;
- strvec_pushl(&child.args, "diff", "--raw", "--no-abbrev", "-z",
+ child->no_stdin = 1;
+ child->git_cmd = 1;
+ child->use_shell = 0;
+ child->clean_on_exit = 1;
+ child->dir = prefix;
+ child->out = -1;
+ strvec_pushl(&child->args, "diff", "--raw", "--no-abbrev", "-z",
NULL);
for (i = 0; i < argc; i++)
- strvec_push(&child.args, argv[i]);
- if (start_command(&child))
+ strvec_push(&child->args, argv[i]);
+ if (start_command(child))
die("could not obtain raw diff");
- fp = xfdopen(child.out, "r");
+ fp = xfdopen(child->out, "r");
/* Build index info for left and right sides of the diff */
i = 0;
@@ -525,7 +525,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
fclose(fp);
fp = NULL;
- if (finish_command(&child)) {
+ if (finish_command(child)) {
ret = error("error occurred running diff --raw");
goto finish;
}
@@ -719,6 +719,7 @@ int cmd_difftool(int argc, const char **argv, const char *prefix)
OPT_ARGUMENT("no-index", &no_index, N_("passed to `diff`")),
OPT_END()
};
+ struct child_process child = CHILD_PROCESS_INIT;
git_config(difftool_config, NULL);
symlinks = has_symlinks;
@@ -769,6 +770,6 @@ int cmd_difftool(int argc, const char **argv, const char *prefix)
* each file that changed.
*/
if (dir_diff)
- return run_dir_diff(extcmd, symlinks, prefix, argc, argv);
+ return run_dir_diff(extcmd, symlinks, prefix, argc, argv, &child);
return run_file_diff(prompt, prefix, argc, argv);
}
From b4c7aab7b9fa7f97c6328751b2cc429189b08514 Mon Sep 17 00:00:00 2001
From: Jeff King <peff@peff.net>
Date: Mon, 13 Sep 2021 05:35:38 +0200
Subject: [PATCH 2/4] difftool: prepare "diff" cmdline in cmd_difftool()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
We call into either run_dir_diff() or run_file_diff(), each of which
sets up a child argv starting with "diff" and some hard-coded options
(depending on which mode we're using). Let's extract that logic into the
caller, which will make it easier to modify the options for cases which
affect both functions.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/difftool.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/builtin/difftool.c b/builtin/difftool.c
index 9f08a8f3fd..f8fcc67640 100644
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@ -331,7 +331,6 @@ static int checkout_path(unsigned mode, struct object_id *oid,
}
static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
- int argc, const char **argv,
struct child_process *child)
{
char tmpdir[PATH_MAX];
@@ -393,10 +392,6 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,
child->clean_on_exit = 1;
child->dir = prefix;
child->out = -1;
- strvec_pushl(&child->args, "diff", "--raw", "--no-abbrev", "-z",
- NULL);
- for (i = 0; i < argc; i++)
- strvec_push(&child->args, argv[i]);
if (start_command(child))
die("could not obtain raw diff");
fp = xfdopen(child->out, "r");
@@ -683,7 +678,6 @@ static int run_file_diff(int prompt, const char *prefix,
env[2] = "GIT_DIFFTOOL_NO_PROMPT=true";
- strvec_push(&args, "diff");
for (i = 0; i < argc; i++)
strvec_push(&args, argv[i]);
return run_command_v_opt_cd_env(args.v, RUN_GIT_CMD, prefix, env);
@@ -769,7 +763,12 @@ int cmd_difftool(int argc, const char **argv, const char *prefix)
* will invoke a separate instance of 'git-difftool--helper' for
* each file that changed.
*/
+ strvec_push(&child.args, "diff");
if (dir_diff)
- return run_dir_diff(extcmd, symlinks, prefix, argc, argv, &child);
- return run_file_diff(prompt, prefix, argc, argv);
+ strvec_pushl(&child.args, "--raw", "--no-abbrev", "-z", NULL);
+ strvec_pushv(&child.args, argv);
+
+ if (dir_diff)
+ return run_dir_diff(extcmd, symlinks, prefix, &child);
+ return run_file_diff(prompt, prefix, child.args.nr, child.args.v);
}
From cc5b594788c3fc89ab5e84de2d657ddf36409821 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?=
<avarab@gmail.com>
Date: Mon, 13 Sep 2021 05:35:39 +0200
Subject: [PATCH 3/4] difftool: use run_command() API in run_file_diff()
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Change the run_file_diff() function to use the run_command() API
directly, instead of invoking the run_command_v_opt_cd_env() wrapper.
This allows it, like run_dir_diff(), to use the "args" from "struct
strvec", instead of the "const char **argv" passed into
cmd_difftool(). This will be used in the subsequent commit to get rid
of OPT_ARGUMENT() from cmd_difftool().
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/difftool.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/builtin/difftool.c b/builtin/difftool.c
index f8fcc67640..de2e5545c8 100644
--- a/builtin/difftool.c
+++ b/builtin/difftool.c
@@ -663,24 +663,23 @@ finish:
}
static int run_file_diff(int prompt, const char *prefix,
- int argc, const char **argv)
+ struct child_process *child)
{
- struct strvec args = STRVEC_INIT;
const char *env[] = {
"GIT_PAGER=", "GIT_EXTERNAL_DIFF=git-difftool--helper", NULL,
NULL
};
- int i;
if (prompt > 0)
env[2] = "GIT_DIFFTOOL_PROMPT=true";
else if (!prompt)
env[2] = "GIT_DIFFTOOL_NO_PROMPT=true";
+ child->git_cmd = 1;
+ child->dir = prefix;
+ strvec_pushv(&child->env_array, env);
- for (i = 0; i < argc; i++)
- strvec_push(&args, argv[i]);
- return run_command_v_opt_cd_env(args.v, RUN_GIT_CMD, prefix, env);
+ return run_command(child);
}
int cmd_difftool(int argc, const char **argv, const char *prefix)
@@ -770,5 +769,5 @@ int cmd_difftool(int argc, const char **argv, const char *prefix)
if (dir_diff)
return run_dir_diff(extcmd, symlinks, prefix, &child);
- return run_file_diff(prompt, prefix, child.args.nr, child.args.v);
+ return run_file_diff(prompt, prefix, &child);
}
From 4c25356e0edaa92e21aadb67579a0263019fbdc4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?=
<avarab@gmail.com>
Date: Mon, 13 Sep 2021 05:35:40 +0200
Subject: [PATCH 4/4] parse-options API: remove OPTION_ARGUMENT feature
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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>
---
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 5a60bbfa7f..acfd5dc1d8 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 de2e5545c8..bb9fe7245a 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 2abff136a1..55c5821b08 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 a845a9d952..39d9088254 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 2051ce57db..a282b6ff13 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 ad4746d899..da310ed29b 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