[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

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

[Junio C Hamano]

Tanga Joris <tangajoris@gmail.com> writes:

> 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);

It is way too ugly to target this so specifically.

I wonder if we can teach show_usage_with_options_if_asked() to do
this?  With this, and the above reverted, the new test added to
t9300 in this patch seems to pass.

 parse-options.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git c/parse-options.c w/parse-options.c
index 283a0f1dc2..f2a23b79a4 100644
--- c/parse-options.c
+++ w/parse-options.c
@@ -1536,6 +1536,12 @@ void show_usage_with_options_if_asked(int ac, const char **av,
 			usage_with_options_internal(NULL, usagestr, opts,
 						    USAGE_FULL, USAGE_TO_STDOUT);
 			exit(0);
+		} else if (!strcmp(av[1], "--git-completion-helper")) {
+			show_gitcomp(opts, 0);
+			exit(0);
+		} else if (!strcmp(av[1], "--git-completion-helper-all")) {
+			show_gitcomp(opts, 1);
+			exit(0);
 		}
 	}
 }

[Outreachy PATCH v3 1/2] parse-options: answer --git-completion-helper before command runs

[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.

Since show_usage_with_options_if_asked() already answers -h and
--help-all before a command does any work, it now also answers
the completion helper, so fast-import and other commands that
call it reply without reading input.

Reported-by: René Scharfe <l.s.r@web.de>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Tanga Joris <tangajoris@gmail.com>
---
Changes since v2:
 - Moved the check from fast-import into
   show_usage_with_options_if_asked() in parse-options,
   as Junio suggested.
 - 2/2 is unchanged

 parse-options.c        | 6 ++++++
 t/t9300-fast-import.sh | 8 ++++++++
 2 files changed, 14 insertions(+)

diff --git a/parse-options.c b/parse-options.c
index 283a0f1dc2..f2a23b79a4 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -1536,6 +1536,12 @@ void show_usage_with_options_if_asked(int ac, const char **av,
 			usage_with_options_internal(NULL, usagestr, opts,
 						    USAGE_FULL, USAGE_TO_STDOUT);
 			exit(0);
+		} else if (!strcmp(av[1], "--git-completion-helper")) {
+			show_gitcomp(opts, 0);
+			exit(0);
+		} else if (!strcmp(av[1], "--git-completion-helper-all")) {
+			show_gitcomp(opts, 1);
+			exit(0);
 		}
 	}
 }
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.43.0

[Outreachy PATCH v3 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.43.0

Re: [Outreachy PATCH v3 1/2] parse-options: answer --git-completion-helper before command runs

[Kaartic Sivaraam]

Hi Tanga Joris,

Just a gentle reminder about this portion of the MyFirstContribution[1]:

   It’s good manners to reply to each comment in the mailing list
   discussion instead of letting the next version of your patch be your
   only response. Tell the reviewer whether you plan to make the
   suggested change, keep the original, or pursue a different approach.
   This way reviewers can respond to your reasoning before you spend time
   preparing a version they may not agree with, and later do not need to
   inspect your v2 to figure out whether you implemented their comment or
   not.

On 10/30/26 04:46, Tanga Joris wrote:
> 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.
> 

May be give a hint that you plan to do this in the following commit? 
That might give someone without context (for example someone reading the 
commit after a while) a hint about why you are worrying about marking 
fast-import with the NO_PARSEOPT flag.

> Since show_usage_with_options_if_asked() already answers -h and
> --help-all before a command does any work, it now also answers
> the completion helper, so fast-import and other commands that
> call it reply without reading input.
> 
> Reported-by: René Scharfe <l.s.r@web.de>
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Tanga Joris <tangajoris@gmail.com>
> ---
> Changes since v2:
>   - Moved the check from fast-import into
>     show_usage_with_options_if_asked() in parse-options,
>     as Junio suggested.
>   - 2/2 is unchanged
> 
>   parse-options.c        | 6 ++++++
>   t/t9300-fast-import.sh | 8 ++++++++
>   2 files changed, 14 insertions(+)
> 
> diff --git a/parse-options.c b/parse-options.c
> index 283a0f1dc2..f2a23b79a4 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -1536,6 +1536,12 @@ void show_usage_with_options_if_asked(int ac, const char **av,
>   			usage_with_options_internal(NULL, usagestr, opts,
>   						    USAGE_FULL, USAGE_TO_STDOUT);
>   			exit(0);
> +		} else if (!strcmp(av[1], "--git-completion-helper")) {
> +			show_gitcomp(opts, 0);
> +			exit(0);
> +		} else if (!strcmp(av[1], "--git-completion-helper-all")) {
> +			show_gitcomp(opts, 1);
> +			exit(0);
>   		}
>   	}
>   }

The change seems to be fine but I wonder if it is worth a note in 
parse-options.h about why show_usage_with_options_if_asked() is also 
handling the '--git-completion-helper' and '--git-completion-helper-all' 
arguments?

[ References ]

[1]: https://git-scm.com/docs/MyFirstContribution#reviewing

-- 
Sivaraam

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

[Kaartic Sivaraam]

On 10/30/26 04:46, Tanga Joris wrote:
> 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 },

This looks good to me. I was wondering if it would be valuable to also 
add a test so that any future commands that start using parse_options() 
could be caught if the corresponding change does not also remove 
NO_PARSEOPT from the command?

For instance, something like so:
         '
diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index b0a3fdbbbc..cb147f30ef 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -267,4 +267,29 @@ do
  	'
  done <builtins

+test_expect_success 'generate list of builtins marked NO_PARSEOPT' '
+	git --list-cmds=parseopt | tr " " "\n" >parseopt &&
+	sort builtins >sorted-builtins &&
+	sort parseopt >sorted-parseopt &&
+	false &&
+	comm -23 sorted-builtins sorted-parseopt >no-parseopt
+'
+
+while read builtin
+do
+	case "$builtin" in
+	# these parse their own options by hand, but pass the diff
+	# options through parse-options, which answers the helper
+	diff|diff-files|diff-index|diff-tree|rev-list)
+		continue ;;
+	esac
+
+	test_expect_success "$builtin is not using parse-options" '
+		test_might_fail git $builtin --git-completion-helper \
+			</dev/null >output 2>/dev/null &&
+		# rev-parse echoes unknown arguments back
+		! grep -v "^--git-completion-helper\$" output | grep "^--"
+	'
+done <no-parseopt
+
  test_done

-- 
Sivaraam

Re: [Outreachy PATCH v3 1/2] parse-options: answer --git-completion-helper before command runs

[Tanga Joris]

Hello Kaartic,

Thank you so much for taking the time to review this.

On Fri, 30 Oct 2026 at 12:44, Kaartic Sivaraam
<kaartic.sivaraam@gmail.com> wrote:
> Just a gentle reminder about this portion of the MyFirstContribution[1]:
>
>    It’s good manners to reply to each comment in the mailing list
>    discussion instead of letting the next version of your patch be your
>    only response. Tell the reviewer whether you plan to make the
>    suggested change, keep the original, or pursue a different approach.

Thanks for the reminder about replying to comments. I'll do that from
now on.

> May be give a hint that you plan to do this in the following commit?
> That might give someone without context (for example someone reading the
> commit after a while) a hint about why you are worrying about marking
> fast-import with the NO_PARSEOPT flag.

Alright, I'll add a sentence to the commit message saying that the
next commit removes the NO_PARSEOPT flag from fast-import, which would
make "git fast-import --<TAB>" freeze without this patch.

> The change seems to be fine but I wonder if it is worth a note in
> parse-options.h about why show_usage_with_options_if_asked() is also
> handling the '--git-completion-helper' and '--git-completion-helper-all'
> arguments?

Yes, I think that would help, so I will add a short comment above its
declaration in parse-options.h explaining that it answers these before
the command does any work.

Thanks,
Tanga

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

[Tanga Joris]

Hello Kaartic,

Thanks for reviewing my patch 2/2, and for writing the test sketch.

On Fri, 30 Oct 2026 at 13:21, Kaartic Sivaraam
<kaartic.sivaraam@gmail.com> wrote:
> This looks good to me. I was wondering if it would be valuable to also
> add a test so that any future commands that start using parse_options()
> could be caught if the corresponding change does not also remove
> NO_PARSEOPT from the command?

I also think the test is a good idea. It would definitely catch future commands
that switch to parse-options but forget to remove NO_PARSEOPT.

I tried your sketch on top of this series. Without 2/2, it fails for
diff-pairs, fast-import and patch-id, so it catches the bug this
patch fixes. With patch 2/2, it passes. I had to remove the
'false &&' line to make it run, which I think was left over from testing.

It does not catch var, though, because var only has short options
(like -l), and the test looks for lines starting with '--'.

Currently, I am not sure how to make the test catch commands with only short
options, because the helper only lists those with long options. Do you
have an idea on how we could do this?

Would you like the test inside 2/2, or as a separate patch 3/3?

Thanks,
Tanga

Re: [Outreachy PATCH v3 1/2] parse-options: answer --git-completion-helper before command runs

[Cynthia Uche]

Hi Tanga,

I applied v3 1/2 and 2/2 on top of master (ebdc087dc9) and built with
NO_RUST=1. t9300, t0012 and t9902 pass. On master,
"git --list-cmds=parseopt" lists none of diff-pairs, fast-import,
patch-id and var; with the series it lists all four, and the first
three print their options with --git-completion-helper.

On Fri, Oct 30, 2026 at 12:17 AM Tanga Joris <tangajoris@gmail.com> wrote:
> +		} else if (!strcmp(av[1], "--git-completion-helper-all")) {
> +			show_gitcomp(opts, 1);
> +			exit(0);

Since this affects every caller of show_usage_with_options_if_asked(),
I ran "git <cmd> --git-completion-helper" and the "-all" form with
</dev/null for every builtin in an empty repository, with builds of
master and of the series, and compared stdout, stderr and the exit
status. They were identical.

The new t9300 test fails without the parse-options.c change, so it
guards the fix, but it only covers the plain form: with the "-all"
branch changed to show_gitcomp(opts, 0) it still passes. Could you
add a test for that branch? This one fails with that change and
passes without it:

  test_expect_success 'R: --git-completion-helper-all does not read from stdin' '
  	echo garbage | git fast-import --git-completion-helper-all >all &&
  	test_grep -e "--allow-unsafe-features" all
  '

Cynthia

Re: [Outreachy PATCH v3 1/2] parse-options: answer --git-completion-helper before command runs

[Tanga Joris]

Hey Cynthia,

Thank you for testing this.

On Tue, 3 Nov 2026 at 12:25, Cynthia Uche <herroyalpriestess@gmail.com> wrote:
> The new t9300 test fails without the parse-options.c change, so it
> guards the fix, but it only covers the plain form: with the "-all"
> branch changed to show_gitcomp(opts, 0) it still passes. Could you
> add a test for that branch?

Yes, I will add the test in my v4. I have tried it locally myself and
it passes with my fix
and fails when the -all branch is broken. Thanks again, I will credit
you with Helped-by:.

Tanga

[Outreachy PATCH v4 1/3] parse-options: answer --git-completion-helper before command runs

[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. The next
commit removes the NO_PARSEOPT flag from fast-import, which would make
"git fast-import --<TAB>" freeze without this patch.

show_usage_with_options_if_asked() already answers -h and --help-all
before a command does any work. Teach it to answer the completion
helper too. Then fast-import, and the other commands that call it,
reply without reading input.

Reported-by: René Scharfe <l.s.r@web.de>
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Helped-by: Cynthia Uche <herroyalpriestess@gmail.com>
Signed-off-by: Tanga Joris <tangajoris@gmail.com>
---
Changes since v3:
 - Added a sentence to the log message saying that the next commit
   removes NO_PARSEOPT from fast-import, as Kaartic suggested.
 - Added a comment above show_usage_with_options_if_asked() in
   parse-options.h, as Kaartic suggested.
 - Added Cynthia's test for --git-completion-helper-all. I also added
   the tmp_pack_* check from the first test to it, because without
   it the test still passes when the fix is reverted.
 - 2/3 is unchanged.
 - 3/3 is new, it adds the test Kaartic suggested.

 parse-options.c        |  6 ++++++
 parse-options.h        |  6 ++++++
 t/t9300-fast-import.sh | 16 ++++++++++++++++
 3 files changed, 28 insertions(+)

diff --git a/parse-options.c b/parse-options.c
index 283a0f1dc2..f2a23b79a4 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -1536,6 +1536,12 @@ void show_usage_with_options_if_asked(int ac, const char **av,
 			usage_with_options_internal(NULL, usagestr, opts,
 						    USAGE_FULL, USAGE_TO_STDOUT);
 			exit(0);
+		} else if (!strcmp(av[1], "--git-completion-helper")) {
+			show_gitcomp(opts, 0);
+			exit(0);
+		} else if (!strcmp(av[1], "--git-completion-helper-all")) {
+			show_gitcomp(opts, 1);
+			exit(0);
 		}
 	}
 }
diff --git a/parse-options.h b/parse-options.h
index f68b51ceca..7ad33801fe 100644
--- a/parse-options.h
+++ b/parse-options.h
@@ -427,6 +427,12 @@ int parse_options(int argc, const char **argv, const char *prefix,
 NORETURN void usage_with_options(const char * const *usagestr,
 				 const struct option *options);
 
+/*
+ * If the only argument is -h, --help-all, --git-completion-helper or
+ * --git-completion-helper-all, print the answer and exit. Call this
+ * before the command does any work. Then asking for help or for the
+ * completion list does not wait for input or leave files behind.
+ */
 void show_usage_with_options_if_asked(int ac, const char **av,
 				      const char * const *usage,
 				      const struct option *options);
diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh
index 73c6e3d448..1305614381 100755
--- a/t/t9300-fast-import.sh
+++ b/t/t9300-fast-import.sh
@@ -2914,6 +2914,22 @@ 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
+'
+
+test_expect_success 'R: --git-completion-helper-all does not read from stdin' '
+	find .git/objects/pack -name "tmp_pack_*" >before &&
+	echo garbage | git fast-import --git-completion-helper-all >all &&
+	test_grep -e "--allow-unsafe-features" all &&
+	find .git/objects/pack -name "tmp_pack_*" >after &&
+	test_cmp before after
+'
+
 ###
 ### series S (mark and path parsing)
 ###
-- 
2.43.0

[Outreachy PATCH v4 2/3] 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.43.0

[Outreachy PATCH v4 3/3] t0012: catch builtins that keep NO_PARSEOPT after conversion

[Tanga Joris]

When a builtin is converted to parse-options, the NO_PARSEOPT flag in
git.c has to be removed by hand. No test checks this. The previous
commit removes the flag from four builtins where it was left behind.

Add a test that runs "git <cmd> --git-completion-helper" for every
builtin left out of "git --list-cmds=parseopt". The test fails if the
command prints an option starting with "--". Without the previous
commit, it fails for diff-pairs, fast-import and patch-id.

The test skips diff, diff-files, diff-index, diff-tree and rev-list,
because they print the diff options. It also skips rev-parse, because
it prints the argument back. It does not catch var, because var only
has short options (like -l) and the helper only lists long options.

Suggested-by: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Signed-off-by: Tanga Joris <tangajoris@gmail.com>
---
Kaartic, I put your test in a separate patch for now, so it is easy to
find. I'm happy to squash it into 2/3 if you prefer it there.

 t/t0012-help.sh | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/t/t0012-help.sh b/t/t0012-help.sh
index b0a3fdbbbc..41197c67e2 100755
--- a/t/t0012-help.sh
+++ b/t/t0012-help.sh
@@ -267,4 +267,29 @@ do
 	'
 done <builtins
 
+test_expect_success 'generate list of builtins without parse-options' '
+	git --list-cmds=parseopt >parseopt &&
+	tr " " "\n" <parseopt | sort >sorted-parseopt &&
+	sort builtins >sorted-builtins &&
+	comm -23 sorted-builtins sorted-parseopt >no-parseopt
+'
+
+while read builtin
+do
+	case "$builtin" in
+	# these answer with the diff options
+	diff|diff-files|diff-index|diff-tree|rev-list)
+		continue ;;
+	# rev-parse prints unknown arguments back
+	rev-parse)
+		continue ;;
+	esac
+
+	test_expect_success "$builtin does not answer --git-completion-helper" '
+		test_might_fail git $builtin --git-completion-helper \
+			</dev/null >output 2>/dev/null &&
+		test_grep ! "^--" output
+	'
+done <no-parseopt
+
 test_done
-- 
2.43.0

Re: [Outreachy PATCH v4 1/3] parse-options: answer --git-completion-helper before command runs

[Junio C Hamano]

Tanga Joris <tangajoris@gmail.com> writes:

> diff --git a/parse-options.h b/parse-options.h
> index f68b51ceca..7ad33801fe 100644
> --- a/parse-options.h
> +++ b/parse-options.h
> @@ -427,6 +427,12 @@ int parse_options(int argc, const char **argv, const char *prefix,
>  NORETURN void usage_with_options(const char * const *usagestr,
>  				 const struct option *options);
>  
> +/*
> + * If the only argument is -h, --help-all, --git-completion-helper or
> + * --git-completion-helper-all, print the answer and exit. Call this
> + * before the command does any work. Then asking for help or for the
> + * completion list does not wait for input or leave files behind.
> + */

This is a very strange thing to say as a comment for this function.

There is no reason to expect any normal program to "wait for input"
or "leave files behind" unless the program has some strange trait.
What is it?

I think what you want to describe is

 - A program that uses parse_options() early in the program has no
   reason to use this function.

 - If there is a reason why you may have to react to help request
   from the command line before the control hits parse_options(),
   you may want to call this function to react to such a request.


"wait for input" and "leave files behind" are peculiarity that
exists only in fast-import, that is shared with nobody else, no?
The fact that it does not call parse_options() until it gets an
input, and that you want it to react to "-h", is what makes this
function useful in that context.