[Outreachy PATCH] git.c: drop NO_PARSEOPT from builtins that use parse-options

[Outreachy PATCH] git.c: drop NO_PARSEOPT from builtins that use parse-options

[Tanga Joris]

The NO_PARSEOPT flag in the commands[] table marks builtins that
parse their options by hand. Builtins marked with it are left out
of "git --list-cmds=parseopt", which bash completion uses to
decide whether it can ask a command for its options with
"--git-completion-helper".

Four builtins have since been converted to parse-options, but are
still marked NO_PARSEOPT:

 - patch-id, in 2871f4d447 (builtin: patch-id: add --verbatim as a
   command mode, 2022-10-24)
 - diff-pairs, which has used parse-options since it was introduced
   in 5bd10b2adc (builtin: introduce diff-pairs command, 2025-02-28)
 - fast-import, in 863937696e (fast-import: use parse_options() for
   command line options, 2026-08-11)
 - var, in e19f0779c6 (var: add "-z" output mode, 2026-09-26)

This causes "git fast-import --<TAB>", "git patch-id --<TAB>" and
"git diff-pairs --<TAB>" to offer no options, even though they are
listed by "--git-completion-helper".

Remove the flag from these four builtins, like 66fa6e8ed8 (git.c:
update NO_PARSEOPT markings, 2022-08-19) did for others.

Signed-off-by: Tanga Joris <tangajoris@gmail.com>
---
Removing the flag from var does not change its completion, since
it only has short options: -l and -z. rev-parse also calls
parse_options(), but unlike the four above, parse_options() is
called only in its --parseopt mode. Its normal options are parsed
by hand, so it keeps the flag.

 git.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/git.c b/git.c
index d7b5f88e52..2d27ad4175 100644
--- a/git.c
+++ b/git.c
@@ -571,11 +571,11 @@ static struct cmd_struct commands[] = {
 	{ "diff", cmd_diff, NO_PARSEOPT },
 	{ "diff-files", cmd_diff_files, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT },
 	{ "diff-index", cmd_diff_index, RUN_SETUP | NO_PARSEOPT },
-	{ "diff-pairs", cmd_diff_pairs, RUN_SETUP | NO_PARSEOPT },
+	{ "diff-pairs", cmd_diff_pairs, RUN_SETUP },
 	{ "diff-tree", cmd_diff_tree, RUN_SETUP | NO_PARSEOPT },
 	{ "difftool", cmd_difftool, RUN_SETUP_GENTLY },
 	{ "fast-export", cmd_fast_export, RUN_SETUP },
-	{ "fast-import", cmd_fast_import, RUN_SETUP | NO_PARSEOPT },
+	{ "fast-import", cmd_fast_import, RUN_SETUP },
 	{ "fetch", cmd_fetch, RUN_SETUP },
 	{ "fetch-pack", cmd_fetch_pack, RUN_SETUP | NO_PARSEOPT },
 	{ "fmt-merge-msg", cmd_fmt_merge_msg, RUN_SETUP },
@@ -626,7 +626,7 @@ static struct cmd_struct commands[] = {
 	{ "pack-redundant", cmd_pack_redundant, RUN_SETUP | NO_PARSEOPT | DEPRECATED },
 #endif
 	{ "pack-refs", cmd_pack_refs, RUN_SETUP },
-	{ "patch-id", cmd_patch_id, RUN_SETUP_GENTLY | NO_PARSEOPT },
+	{ "patch-id", cmd_patch_id, RUN_SETUP_GENTLY },
 	{ "pickaxe", cmd_blame, RUN_SETUP },
 	{ "prune", cmd_prune, RUN_SETUP },
 	{ "prune-packed", cmd_prune_packed, RUN_SETUP },
@@ -676,7 +676,7 @@ static struct cmd_struct commands[] = {
 	{ "upload-archive--writer", cmd_upload_archive_writer, NO_PARSEOPT },
 	{ "upload-pack", cmd_upload_pack },
 	{ "url-parse", cmd_url_parse },
-	{ "var", cmd_var, RUN_SETUP_GENTLY | NO_PARSEOPT },
+	{ "var", cmd_var, RUN_SETUP_GENTLY },
 	{ "verify-commit", cmd_verify_commit, RUN_SETUP },
 	{ "verify-pack", cmd_verify_pack },
 	{ "verify-tag", cmd_verify_tag, RUN_SETUP },
-- 
2.43.0

Re: [Outreachy PATCH] git.c: drop NO_PARSEOPT from builtins that use parse-options

[René Scharfe]

On 10/24/26 7:24 AM, Tanga Joris wrote:
> The NO_PARSEOPT flag in the commands[] table marks builtins that
> parse their options by hand. Builtins marked with it are left out
> of "git --list-cmds=parseopt", which bash completion uses to
> decide whether it can ask a command for its options with
> "--git-completion-helper".
> 
> Four builtins have since been converted to parse-options, but are
> still marked NO_PARSEOPT:
> 
>  - patch-id, in 2871f4d447 (builtin: patch-id: add --verbatim as a
>    command mode, 2022-10-24)
>  - diff-pairs, which has used parse-options since it was introduced
>    in 5bd10b2adc (builtin: introduce diff-pairs command, 2025-02-28)
>  - fast-import, in 863937696e (fast-import: use parse_options() for
>    command line options, 2026-08-11)
>  - var, in e19f0779c6 (var: add "-z" output mode, 2026-09-26)
> 
> This causes "git fast-import --<TAB>", "git patch-id --<TAB>" and
> "git diff-pairs --<TAB>" to offer no options, even though they are
> listed by "--git-completion-helper".
> 
> Remove the flag from these four builtins, like 66fa6e8ed8 (git.c:
> update NO_PARSEOPT markings, 2022-08-19) did for others.

Good idea!

"git fast-import --git-completion-helper" prints its command line
options only after it gets fed a line of input.  That's odd.  Do the
completion scripts still work seamlessly, i.e. without requiring the
user to press <return>?

René

Re: [Outreachy PATCH] git.c: drop NO_PARSEOPT from builtins that use parse-options

[Tanga Joris]

On Sun, 25 Oct 2026 at 10:59, René Scharfe <l.s.r@web.de> wrote:

> "git fast-import --git-completion-helper" prints its command line
> options only after it gets fed a line of input.  That's odd.  Do the
> completion scripts still work seamlessly, i.e. without requiring the
> user to press <return>?

Thanks for catching this. They don't. __git() in the completion
script does not redirect stdin, so the helper reads from the
terminal and "git fast-import --<TAB>" waits until the user presses
<return> or ^D.

This seems to happen because fast-import only calls parse_options()
once it sees the first command in the stream (or EOF), as it reads
the "feature" lines first.

Since patch-id, diff-pairs and var return immediately, I'll send a v2
that still removes the NO_PARSEOPT flag from them, but keeps it
on fast-import, for now, as I make a separate patch fixing the wait
behaviour, and ultimately removing the flag.

The separate patch would teach fast-import to answer
--git-completion-helper before reading its input, like it does for "-h".

Christian, I plan to base it on top of your early-scan series, since
it touches the same code. Would you be fine with that?

Thanks,
Tanga

Re: [Outreachy PATCH] git.c: drop NO_PARSEOPT from builtins that use parse-options

[Junio C Hamano]

Tanga Joris <tangajoris@gmail.com> writes:

> Christian, I plan to base it on top of your early-scan series, since
> it touches the same code. Would you be fine with that?

It is not something Christian can approve, nor do you need his
permission.

You need to make sure that the topic you plan to depend on is as
solid as possible.  Those who write topics that depend on other
topics usually make sure the topics they depend on are at least in
'next'.

You know how you can help make Christian's 'cc/early-scan-options'
topic as solid as possible and help it get merged to 'next' sooner,
right?  In this community, topics do not move forward without
positive reviews.  As a potential user of the service the topic
offers, you are in a better position than random bystanders to
carefully review it, find issues, and offer improvements.

Thanks.

[Outreachy PATCH v2 1/2] fast-import: handle --git-completion-helper before reading input

[Tanga Joris]

fast-import parses its options only after reading the "feature"
lines from its input, and it creates a temporary pack file before
that. So "git fast-import --git-completion-helper" waits for input,
and each run leaves a tmp_pack_* file behind.

bash completion runs the helper with the terminal as input, so
"git fast-import --<TAB>" would freeze once fast-import is no
longer marked with the NO_PARSEOPT flag.

Handle a lone --git-completion-helper or --git-completion-helper-all
right after the -h check, before creating the pack or reading input.

Reported-by: René Scharfe <l.s.r@web.de>
Signed-off-by: Tanga Joris <tangajoris@gmail.com>
---
Changes since v1:
 - New patch 1/2, addressing René's observation that completion would
   block on fast-import
 - 2/2 is unchanged

 builtin/fast-import.c  | 4 ++++
 t/t9300-fast-import.sh | 8 ++++++++
 2 files changed, 12 insertions(+)

diff --git a/builtin/fast-import.c b/builtin/fast-import.c
index 9db3f83eea..f53113d5a5 100644
--- a/builtin/fast-import.c
+++ b/builtin/fast-import.c
@@ -4204,6 +4204,10 @@ int cmd_fast_import(int argc,
 	};
 
 	show_usage_with_options_if_asked(argc, argv, fast_import_usage, fast_import_options);
+	if (argc == 2 && (!strcmp(argv[1], "--git-completion-helper") ||
+			  !strcmp(argv[1], "--git-completion-helper-all")))
+		parse_options(argc, argv, prefix, fast_import_options,
+			      fast_import_usage, 0);
 
 	fast_import_state_init(&state, argc, argv, prefix, fast_import_options);
 
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 73c6e3d448..99d7843217 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -2914,6 +2914,14 @@ test_expect_success 'R: blob appears only once' '
 	test 1 = $n
 '
 
+test_expect_success 'R: --git-completion-helper does not read from stdin' '
+	find .git/objects/pack -name "tmp_pack_*" >before &&
+	echo garbage | git fast-import --git-completion-helper >out &&
+	test_grep -e "--date-format=" out &&
+	find .git/objects/pack -name "tmp_pack_*" >after &&
+	test_cmp before after
+'
+
 ###
 ### series S (mark and path parsing)
 ###
-- 
2.56.0.291.g0b48760bf4

[Outreachy PATCH v2 2/2] git.c: drop NO_PARSEOPT from builtins that use parse-options

[Tanga Joris]

The NO_PARSEOPT flag in the commands[] table marks builtins that
parse their options by hand. Builtins marked with it are left out
of "git --list-cmds=parseopt", which bash completion uses to
decide whether it can ask a command for its options with
"--git-completion-helper".

Four builtins have since been converted to parse-options, but are
still marked NO_PARSEOPT:

 - patch-id, in 2871f4d447 (builtin: patch-id: add --verbatim as a
   command mode, 2022-10-24)
 - diff-pairs, which has used parse-options since it was introduced
   in 5bd10b2adc (builtin: introduce diff-pairs command, 2025-02-28)
 - fast-import, in 863937696e (fast-import: use parse_options() for
   command line options, 2026-08-11)
 - var, in e19f0779c6 (var: add "-z" output mode, 2026-09-26)

This causes "git fast-import --<TAB>", "git patch-id --<TAB>" and
"git diff-pairs --<TAB>" to offer no options, even though they are
listed by "--git-completion-helper".

Remove the flag from these four builtins, like 66fa6e8ed8 (git.c:
update NO_PARSEOPT markings, 2022-08-19) did for others.

Signed-off-by: Tanga Joris <tangajoris@gmail.com>
---
 git.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/git.c b/git.c
index d7b5f88e52..2d27ad4175 100644
--- a/git.c
+++ b/git.c
@@ -571,11 +571,11 @@ static struct cmd_struct commands[] = {
 	{ "diff", cmd_diff, NO_PARSEOPT },
 	{ "diff-files", cmd_diff_files, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT },
 	{ "diff-index", cmd_diff_index, RUN_SETUP | NO_PARSEOPT },
-	{ "diff-pairs", cmd_diff_pairs, RUN_SETUP | NO_PARSEOPT },
+	{ "diff-pairs", cmd_diff_pairs, RUN_SETUP },
 	{ "diff-tree", cmd_diff_tree, RUN_SETUP | NO_PARSEOPT },
 	{ "difftool", cmd_difftool, RUN_SETUP_GENTLY },
 	{ "fast-export", cmd_fast_export, RUN_SETUP },
-	{ "fast-import", cmd_fast_import, RUN_SETUP | NO_PARSEOPT },
+	{ "fast-import", cmd_fast_import, RUN_SETUP },
 	{ "fetch", cmd_fetch, RUN_SETUP },
 	{ "fetch-pack", cmd_fetch_pack, RUN_SETUP | NO_PARSEOPT },
 	{ "fmt-merge-msg", cmd_fmt_merge_msg, RUN_SETUP },
@@ -626,7 +626,7 @@ static struct cmd_struct commands[] = {
 	{ "pack-redundant", cmd_pack_redundant, RUN_SETUP | NO_PARSEOPT | DEPRECATED },
 #endif
 	{ "pack-refs", cmd_pack_refs, RUN_SETUP },
-	{ "patch-id", cmd_patch_id, RUN_SETUP_GENTLY | NO_PARSEOPT },
+	{ "patch-id", cmd_patch_id, RUN_SETUP_GENTLY },
 	{ "pickaxe", cmd_blame, RUN_SETUP },
 	{ "prune", cmd_prune, RUN_SETUP },
 	{ "prune-packed", cmd_prune_packed, RUN_SETUP },
@@ -676,7 +676,7 @@ static struct cmd_struct commands[] = {
 	{ "upload-archive--writer", cmd_upload_archive_writer, NO_PARSEOPT },
 	{ "upload-pack", cmd_upload_pack },
 	{ "url-parse", cmd_url_parse },
-	{ "var", cmd_var, RUN_SETUP_GENTLY | NO_PARSEOPT },
+	{ "var", cmd_var, RUN_SETUP_GENTLY },
 	{ "verify-commit", cmd_verify_commit, RUN_SETUP },
 	{ "verify-pack", cmd_verify_pack },
 	{ "verify-tag", cmd_verify_tag, RUN_SETUP },
-- 
2.56.0.291.g0b48760bf4