[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