{"thread":{"id":"66649","subject":"[Outreachy PATCH] git.c: drop NO_PARSEOPT from builtins that use parse-options","startedAt":"2026-10-24T05:24:49Z","lastAt":"2026-11-05T00:39:13Z","messageCount":18,"participants":["Tanga Joris","René Scharfe","Junio C Hamano","Kaartic Sivaraam","Cynthia Uche"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"556139","messageId":"20261024052449.95869-1-tangajoris@gmail.com","threadId":"66649","inReplyTo":null,"subject":"[Outreachy PATCH] git.c: drop NO_PARSEOPT from builtins that use parse-options","fromName":"Tanga Joris","fromEmail":"tangajoris@gmail.com","sentAt":"2026-10-24T05:24:49Z","receivedAt":"2026-10-24T05:24:49Z","isPatch":true,"sender":{"key":"tangajoris@gmail.com","avatar":null},"body":"The NO_PARSEOPT flag in the commands[] table marks builtins that\nparse their options by hand. Builtins marked with it are left out\nof \"git --list-cmds=parseopt\", which bash completion uses to\ndecide whether it can ask a command for its options with\n\"--git-completion-helper\".\n\nFour builtins have since been converted to parse-options, but are\nstill marked NO_PARSEOPT:\n\n - patch-id, in 2871f4d447 (builtin: patch-id: add --verbatim as a\n command mode, 2022-10-24)\n - diff-pairs, which has used parse-options since it was introduced\n in 5bd10b2adc (builtin: introduce diff-pairs command, 2025-02-28)\n - fast-import, in 863937696e (fast-import: use parse_options() for\n command line options, 2026-08-11)\n - var, in e19f0779c6 (var: add \"-z\" output mode, 2026-09-26)\n\nThis causes \"git fast-import --\", \"git patch-id --\" and\n\"git diff-pairs --\" to offer no options, even though they are\nlisted by \"--git-completion-helper\".\n\nRemove the flag from these four builtins, like 66fa6e8ed8 (git.c:\nupdate NO_PARSEOPT markings, 2022-08-19) did for others.\n\nSigned-off-by: Tanga Joris \n---\nRemoving the flag from var does not change its completion, since\nit only has short options: -l and -z. rev-parse also calls\nparse_options(), but unlike the four above, parse_options() is\ncalled only in its --parseopt mode. Its normal options are parsed\nby hand, so it keeps the flag.\n\n git.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/git.c b/git.c\nindex d7b5f88e52..2d27ad4175 100644\n--- a/git.c\n+++ b/git.c\n@@ -571,11 +571,11 @@ static struct cmd_struct commands[] = {\n \t{ \"diff\", cmd_diff, NO_PARSEOPT },\n \t{ \"diff-files\", cmd_diff_files, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT },\n \t{ \"diff-index\", cmd_diff_index, RUN_SETUP | NO_PARSEOPT },\n-\t{ \"diff-pairs\", cmd_diff_pairs, RUN_SETUP | NO_PARSEOPT },\n+\t{ \"diff-pairs\", cmd_diff_pairs, RUN_SETUP },\n \t{ \"diff-tree\", cmd_diff_tree, RUN_SETUP | NO_PARSEOPT },\n \t{ \"difftool\", cmd_difftool, RUN_SETUP_GENTLY },\n \t{ \"fast-export\", cmd_fast_export, RUN_SETUP },\n-\t{ \"fast-import\", cmd_fast_import, RUN_SETUP | NO_PARSEOPT },\n+\t{ \"fast-import\", cmd_fast_import, RUN_SETUP },\n \t{ \"fetch\", cmd_fetch, RUN_SETUP },\n \t{ \"fetch-pack\", cmd_fetch_pack, RUN_SETUP | NO_PARSEOPT },\n \t{ \"fmt-merge-msg\", cmd_fmt_merge_msg, RUN_SETUP },\n@@ -626,7 +626,7 @@ static struct cmd_struct commands[] = {\n \t{ \"pack-redundant\", cmd_pack_redundant, RUN_SETUP | NO_PARSEOPT | DEPRECATED },\n #endif\n \t{ \"pack-refs\", cmd_pack_refs, RUN_SETUP },\n-\t{ \"patch-id\", cmd_patch_id, RUN_SETUP_GENTLY | NO_PARSEOPT },\n+\t{ \"patch-id\", cmd_patch_id, RUN_SETUP_GENTLY },\n \t{ \"pickaxe\", cmd_blame, RUN_SETUP },\n \t{ \"prune\", cmd_prune, RUN_SETUP },\n \t{ \"prune-packed\", cmd_prune_packed, RUN_SETUP },\n@@ -676,7 +676,7 @@ static struct cmd_struct commands[] = {\n \t{ \"upload-archive--writer\", cmd_upload_archive_writer, NO_PARSEOPT },\n \t{ \"upload-pack\", cmd_upload_pack },\n \t{ \"url-parse\", cmd_url_parse },\n-\t{ \"var\", cmd_var, RUN_SETUP_GENTLY | NO_PARSEOPT },\n+\t{ \"var\", cmd_var, RUN_SETUP_GENTLY },\n \t{ \"verify-commit\", cmd_verify_commit, RUN_SETUP },\n \t{ \"verify-pack\", cmd_verify_pack },\n \t{ \"verify-tag\", cmd_verify_tag, RUN_SETUP },\n-- \n2.43.0\n\n\n"},{"id":"556207","messageId":"8b60c7f6-4a1c-495a-a594-aaf322279539@web.de","threadId":"66649","inReplyTo":"20261024052449.95869-1-tangajoris@gmail.com","subject":"Re: [Outreachy PATCH] git.c: drop NO_PARSEOPT from builtins that use parse-options","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2026-10-25T07:59:07Z","receivedAt":"2026-10-25T07:59:07Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"On 10/24/26 7:24 AM, Tanga Joris wrote:\n> The NO_PARSEOPT flag in the commands[] table marks builtins that\n> parse their options by hand. Builtins marked with it are left out\n> of \"git --list-cmds=parseopt\", which bash completion uses to\n> decide whether it can ask a command for its options with\n> \"--git-completion-helper\".\n> \n> Four builtins have since been converted to parse-options, but are\n> still marked NO_PARSEOPT:\n> \n> - patch-id, in 2871f4d447 (builtin: patch-id: add --verbatim as a\n> command mode, 2022-10-24)\n> - diff-pairs, which has used parse-options since it was introduced\n> in 5bd10b2adc (builtin: introduce diff-pairs command, 2025-02-28)\n> - fast-import, in 863937696e (fast-import: use parse_options() for\n> command line options, 2026-08-11)\n> - var, in e19f0779c6 (var: add \"-z\" output mode, 2026-09-26)\n> \n> This causes \"git fast-import --\", \"git patch-id --\" and\n> \"git diff-pairs --\" to offer no options, even though they are\n> listed by \"--git-completion-helper\".\n> \n> Remove the flag from these four builtins, like 66fa6e8ed8 (git.c:\n> update NO_PARSEOPT markings, 2022-08-19) did for others.\n\nGood idea!\n\n\"git fast-import --git-completion-helper\" prints its command line\noptions only after it gets fed a line of input. That's odd. Do the\ncompletion scripts still work seamlessly, i.e. without requiring the\nuser to press ?\n\nRené\n\n\n"},{"id":"556244","messageId":"CAKrRMNqtqYiE2YKVky+mHyrkqtW2c_L_DcCen1hyaQa8D0+fWw@mail.gmail.com","threadId":"66649","inReplyTo":"8b60c7f6-4a1c-495a-a594-aaf322279539@web.de","subject":"Re: [Outreachy PATCH] git.c: drop NO_PARSEOPT from builtins that use parse-options","fromName":"Tanga Joris","fromEmail":"tangajoris@gmail.com","sentAt":"2026-10-26T02:59:53Z","receivedAt":"2026-10-26T02:59:53Z","isPatch":true,"sender":{"key":"tangajoris@gmail.com","avatar":null},"body":"On Sun, 25 Oct 2026 at 10:59, René Scharfe wrote:\n\n> \"git fast-import --git-completion-helper\" prints its command line\n> options only after it gets fed a line of input. That's odd. Do the\n> completion scripts still work seamlessly, i.e. without requiring the\n> user to press ?\n\nThanks for catching this. They don't. __git() in the completion\nscript does not redirect stdin, so the helper reads from the\nterminal and \"git fast-import --\" waits until the user presses\n or ^D.\n\nThis seems to happen because fast-import only calls parse_options()\nonce it sees the first command in the stream (or EOF), as it reads\nthe \"feature\" lines first.\n\nSince patch-id, diff-pairs and var return immediately, I'll send a v2\nthat still removes the NO_PARSEOPT flag from them, but keeps it\non fast-import, for now, as I make a separate patch fixing the wait\nbehaviour, and ultimately removing the flag.\n\nThe separate patch would teach fast-import to answer\n--git-completion-helper before reading its input, like it does for \"-h\".\n\nChristian, I plan to base it on top of your early-scan series, since\nit touches the same code. Would you be fine with that?\n\nThanks,\nTanga\n\n"},{"id":"556248","messageId":"xmqq33ttvz0q.fsf@gitster.g","threadId":"66649","inReplyTo":"CAKrRMNqtqYiE2YKVky+mHyrkqtW2c_L_DcCen1hyaQa8D0+fWw@mail.gmail.com","subject":"Re: [Outreachy PATCH] git.c: drop NO_PARSEOPT from builtins that use parse-options","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-10-26T04:58:29Z","receivedAt":"2026-10-26T04:58:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tanga Joris writes:\n\n> Christian, I plan to base it on top of your early-scan series, since\n> it touches the same code. Would you be fine with that?\n\nIt is not something Christian can approve, nor do you need his\npermission.\n\nYou need to make sure that the topic you plan to depend on is as\nsolid as possible. Those who write topics that depend on other\ntopics usually make sure the topics they depend on are at least in\n'next'.\n\nYou know how you can help make Christian's 'cc/early-scan-options'\ntopic as solid as possible and help it get merged to 'next' sooner,\nright? In this community, topics do not move forward without\npositive reviews. As a potential user of the service the topic\noffers, you are in a better position than random bystanders to\ncarefully review it, find issues, and offer improvements.\n\nThanks.\n\n"},{"id":"556722","messageId":"20261029012616.139783-1-tangajoris@gmail.com","threadId":"66649","inReplyTo":"20261024052449.95869-1-tangajoris@gmail.com","subject":"[Outreachy PATCH v2 1/2] fast-import: handle --git-completion-helper before reading input","fromName":"Tanga Joris","fromEmail":"tangajoris@gmail.com","sentAt":"2026-10-29T01:26:15Z","receivedAt":"2026-10-29T01:26:15Z","isPatch":true,"sender":{"key":"tangajoris@gmail.com","avatar":null},"body":"fast-import parses its options only after reading the \"feature\"\nlines from its input, and it creates a temporary pack file before\nthat. So \"git fast-import --git-completion-helper\" waits for input,\nand each run leaves a tmp_pack_* file behind.\n\nbash completion runs the helper with the terminal as input, so\n\"git fast-import --\" would freeze once fast-import is no\nlonger marked with the NO_PARSEOPT flag.\n\nHandle a lone --git-completion-helper or --git-completion-helper-all\nright after the -h check, before creating the pack or reading input.\n\nReported-by: René Scharfe \nSigned-off-by: Tanga Joris \n---\nChanges since v1:\n - New patch 1/2, addressing René's observation that completion would\n block on fast-import\n - 2/2 is unchanged\n\n builtin/fast-import.c | 4 ++++\n t/t9300-fast-import.sh | 8 ++++++++\n 2 files changed, 12 insertions(+)\n\ndiff --git a/builtin/fast-import.c b/builtin/fast-import.c\nindex 9db3f83eea..f53113d5a5 100644\n--- a/builtin/fast-import.c\n+++ b/builtin/fast-import.c\n@@ -4204,6 +4204,10 @@ int cmd_fast_import(int argc,\n \t};\n \n \tshow_usage_with_options_if_asked(argc, argv, fast_import_usage, fast_import_options);\n+\tif (argc == 2 && (!strcmp(argv[1], \"--git-completion-helper\") ||\n+\t\t\t !strcmp(argv[1], \"--git-completion-helper-all\")))\n+\t\tparse_options(argc, argv, prefix, fast_import_options,\n+\t\t\t fast_import_usage, 0);\n \n \tfast_import_state_init(&state, argc, argv, prefix, fast_import_options);\n \ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex 73c6e3d448..99d7843217 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -2914,6 +2914,14 @@ test_expect_success 'R: blob appears only once' '\n \ttest 1 = $n\n '\n \n+test_expect_success 'R: --git-completion-helper does not read from stdin' '\n+\tfind .git/objects/pack -name \"tmp_pack_*\" >before &&\n+\techo garbage | git fast-import --git-completion-helper >out &&\n+\ttest_grep -e \"--date-format=\" out &&\n+\tfind .git/objects/pack -name \"tmp_pack_*\" >after &&\n+\ttest_cmp before after\n+'\n+\n ###\n ### series S (mark and path parsing)\n ###\n-- \n2.56.0.291.g0b48760bf4\n\n\n"},{"id":"556723","messageId":"20261029012616.139783-2-tangajoris@gmail.com","threadId":"66649","inReplyTo":"20261029012616.139783-1-tangajoris@gmail.com","subject":"[Outreachy PATCH v2 2/2] git.c: drop NO_PARSEOPT from builtins that use parse-options","fromName":"Tanga Joris","fromEmail":"tangajoris@gmail.com","sentAt":"2026-10-29T01:26:16Z","receivedAt":"2026-10-29T01:26:16Z","isPatch":true,"sender":{"key":"tangajoris@gmail.com","avatar":null},"body":"The NO_PARSEOPT flag in the commands[] table marks builtins that\nparse their options by hand. Builtins marked with it are left out\nof \"git --list-cmds=parseopt\", which bash completion uses to\ndecide whether it can ask a command for its options with\n\"--git-completion-helper\".\n\nFour builtins have since been converted to parse-options, but are\nstill marked NO_PARSEOPT:\n\n - patch-id, in 2871f4d447 (builtin: patch-id: add --verbatim as a\n command mode, 2022-10-24)\n - diff-pairs, which has used parse-options since it was introduced\n in 5bd10b2adc (builtin: introduce diff-pairs command, 2025-02-28)\n - fast-import, in 863937696e (fast-import: use parse_options() for\n command line options, 2026-08-11)\n - var, in e19f0779c6 (var: add \"-z\" output mode, 2026-09-26)\n\nThis causes \"git fast-import --\", \"git patch-id --\" and\n\"git diff-pairs --\" to offer no options, even though they are\nlisted by \"--git-completion-helper\".\n\nRemove the flag from these four builtins, like 66fa6e8ed8 (git.c:\nupdate NO_PARSEOPT markings, 2022-08-19) did for others.\n\nSigned-off-by: Tanga Joris \n---\n git.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/git.c b/git.c\nindex d7b5f88e52..2d27ad4175 100644\n--- a/git.c\n+++ b/git.c\n@@ -571,11 +571,11 @@ static struct cmd_struct commands[] = {\n \t{ \"diff\", cmd_diff, NO_PARSEOPT },\n \t{ \"diff-files\", cmd_diff_files, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT },\n \t{ \"diff-index\", cmd_diff_index, RUN_SETUP | NO_PARSEOPT },\n-\t{ \"diff-pairs\", cmd_diff_pairs, RUN_SETUP | NO_PARSEOPT },\n+\t{ \"diff-pairs\", cmd_diff_pairs, RUN_SETUP },\n \t{ \"diff-tree\", cmd_diff_tree, RUN_SETUP | NO_PARSEOPT },\n \t{ \"difftool\", cmd_difftool, RUN_SETUP_GENTLY },\n \t{ \"fast-export\", cmd_fast_export, RUN_SETUP },\n-\t{ \"fast-import\", cmd_fast_import, RUN_SETUP | NO_PARSEOPT },\n+\t{ \"fast-import\", cmd_fast_import, RUN_SETUP },\n \t{ \"fetch\", cmd_fetch, RUN_SETUP },\n \t{ \"fetch-pack\", cmd_fetch_pack, RUN_SETUP | NO_PARSEOPT },\n \t{ \"fmt-merge-msg\", cmd_fmt_merge_msg, RUN_SETUP },\n@@ -626,7 +626,7 @@ static struct cmd_struct commands[] = {\n \t{ \"pack-redundant\", cmd_pack_redundant, RUN_SETUP | NO_PARSEOPT | DEPRECATED },\n #endif\n \t{ \"pack-refs\", cmd_pack_refs, RUN_SETUP },\n-\t{ \"patch-id\", cmd_patch_id, RUN_SETUP_GENTLY | NO_PARSEOPT },\n+\t{ \"patch-id\", cmd_patch_id, RUN_SETUP_GENTLY },\n \t{ \"pickaxe\", cmd_blame, RUN_SETUP },\n \t{ \"prune\", cmd_prune, RUN_SETUP },\n \t{ \"prune-packed\", cmd_prune_packed, RUN_SETUP },\n@@ -676,7 +676,7 @@ static struct cmd_struct commands[] = {\n \t{ \"upload-archive--writer\", cmd_upload_archive_writer, NO_PARSEOPT },\n \t{ \"upload-pack\", cmd_upload_pack },\n \t{ \"url-parse\", cmd_url_parse },\n-\t{ \"var\", cmd_var, RUN_SETUP_GENTLY | NO_PARSEOPT },\n+\t{ \"var\", cmd_var, RUN_SETUP_GENTLY },\n \t{ \"verify-commit\", cmd_verify_commit, RUN_SETUP },\n \t{ \"verify-pack\", cmd_verify_pack },\n \t{ \"verify-tag\", cmd_verify_tag, RUN_SETUP },\n-- \n2.56.0.291.g0b48760bf4\n\n\n"},{"id":"556809","messageId":"xmqqy0bg2xj5.fsf@gitster.g","threadId":"66649","inReplyTo":"20261029012616.139783-1-tangajoris@gmail.com","subject":"Re: [Outreachy PATCH v2 1/2] fast-import: handle --git-completion-helper before reading input","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-10-29T18:04:46Z","receivedAt":"2026-10-29T18:04:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tanga Joris writes:\n\n> diff --git a/builtin/fast-import.c b/builtin/fast-import.c\n> index 9db3f83eea..f53113d5a5 100644\n> --- a/builtin/fast-import.c\n> +++ b/builtin/fast-import.c\n> @@ -4204,6 +4204,10 @@ int cmd_fast_import(int argc,\n> \t};\n> \n> \tshow_usage_with_options_if_asked(argc, argv, fast_import_usage, fast_import_options);\n> +\tif (argc == 2 && (!strcmp(argv[1], \"--git-completion-helper\") ||\n> +\t\t\t !strcmp(argv[1], \"--git-completion-helper-all\")))\n> +\t\tparse_options(argc, argv, prefix, fast_import_options,\n> +\t\t\t fast_import_usage, 0);\n> \n> \tfast_import_state_init(&state, argc, argv, prefix, fast_import_options);\n\nIt is way too ugly to target this so specifically.\n\nI wonder if we can teach show_usage_with_options_if_asked() to do\nthis? With this, and the above reverted, the new test added to\nt9300 in this patch seems to pass.\n\n parse-options.c | 6 ++++++\n 1 file changed, 6 insertions(+)\n\ndiff --git c/parse-options.c w/parse-options.c\nindex 283a0f1dc2..f2a23b79a4 100644\n--- c/parse-options.c\n+++ w/parse-options.c\n@@ -1536,6 +1536,12 @@ void show_usage_with_options_if_asked(int ac, const char **av,\n \t\t\tusage_with_options_internal(NULL, usagestr, opts,\n \t\t\t\t\t\t USAGE_FULL, USAGE_TO_STDOUT);\n \t\t\texit(0);\n+\t\t} else if (!strcmp(av[1], \"--git-completion-helper\")) {\n+\t\t\tshow_gitcomp(opts, 0);\n+\t\t\texit(0);\n+\t\t} else if (!strcmp(av[1], \"--git-completion-helper-all\")) {\n+\t\t\tshow_gitcomp(opts, 1);\n+\t\t\texit(0);\n \t\t}\n \t}\n }\n\n"},{"id":"556844","messageId":"20261029231621.48958-1-tangajoris@gmail.com","threadId":"66649","inReplyTo":"20261029012616.139783-1-tangajoris@gmail.com","subject":"[Outreachy PATCH v3 1/2] parse-options: answer --git-completion-helper before command runs","fromName":"Tanga Joris","fromEmail":"tangajoris@gmail.com","sentAt":"2026-10-29T23:16:20Z","receivedAt":"2026-10-29T23:16:20Z","isPatch":true,"sender":{"key":"tangajoris@gmail.com","avatar":null},"body":"fast-import parses its options only after reading the \"feature\"\nlines from its input, and it creates a temporary pack file before\nthat. So \"git fast-import --git-completion-helper\" waits for input,\nand each run leaves a tmp_pack_* file behind.\n\nbash completion runs the helper with the terminal as input, so\n\"git fast-import --\" would freeze once fast-import is no\nlonger marked with the NO_PARSEOPT flag.\n\nSince show_usage_with_options_if_asked() already answers -h and\n--help-all before a command does any work, it now also answers\nthe completion helper, so fast-import and other commands that\ncall it reply without reading input.\n\nReported-by: René Scharfe \nHelped-by: Junio C Hamano \nSigned-off-by: Tanga Joris \n---\nChanges since v2:\n - Moved the check from fast-import into\n show_usage_with_options_if_asked() in parse-options,\n as Junio suggested.\n - 2/2 is unchanged\n\n parse-options.c | 6 ++++++\n t/t9300-fast-import.sh | 8 ++++++++\n 2 files changed, 14 insertions(+)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex 283a0f1dc2..f2a23b79a4 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -1536,6 +1536,12 @@ void show_usage_with_options_if_asked(int ac, const char **av,\n \t\t\tusage_with_options_internal(NULL, usagestr, opts,\n \t\t\t\t\t\t USAGE_FULL, USAGE_TO_STDOUT);\n \t\t\texit(0);\n+\t\t} else if (!strcmp(av[1], \"--git-completion-helper\")) {\n+\t\t\tshow_gitcomp(opts, 0);\n+\t\t\texit(0);\n+\t\t} else if (!strcmp(av[1], \"--git-completion-helper-all\")) {\n+\t\t\tshow_gitcomp(opts, 1);\n+\t\t\texit(0);\n \t\t}\n \t}\n }\ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex 73c6e3d448..99d7843217 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -2914,6 +2914,14 @@ test_expect_success 'R: blob appears only once' '\n \ttest 1 = $n\n '\n \n+test_expect_success 'R: --git-completion-helper does not read from stdin' '\n+\tfind .git/objects/pack -name \"tmp_pack_*\" >before &&\n+\techo garbage | git fast-import --git-completion-helper >out &&\n+\ttest_grep -e \"--date-format=\" out &&\n+\tfind .git/objects/pack -name \"tmp_pack_*\" >after &&\n+\ttest_cmp before after\n+'\n+\n ###\n ### series S (mark and path parsing)\n ###\n-- \n2.43.0\n\n\n"},{"id":"556845","messageId":"20261029231621.48958-2-tangajoris@gmail.com","threadId":"66649","inReplyTo":"20261029231621.48958-1-tangajoris@gmail.com","subject":"[Outreachy PATCH v3 2/2] git.c: drop NO_PARSEOPT from builtins that use parse-options","fromName":"Tanga Joris","fromEmail":"tangajoris@gmail.com","sentAt":"2026-10-29T23:16:21Z","receivedAt":"2026-10-29T23:16:21Z","isPatch":true,"sender":{"key":"tangajoris@gmail.com","avatar":null},"body":"The NO_PARSEOPT flag in the commands[] table marks builtins that\nparse their options by hand. Builtins marked with it are left out\nof \"git --list-cmds=parseopt\", which bash completion uses to\ndecide whether it can ask a command for its options with\n\"--git-completion-helper\".\n\nFour builtins have since been converted to parse-options, but are\nstill marked NO_PARSEOPT:\n\n - patch-id, in 2871f4d447 (builtin: patch-id: add --verbatim as a\n command mode, 2022-10-24)\n - diff-pairs, which has used parse-options since it was introduced\n in 5bd10b2adc (builtin: introduce diff-pairs command, 2025-02-28)\n - fast-import, in 863937696e (fast-import: use parse_options() for\n command line options, 2026-08-11)\n - var, in e19f0779c6 (var: add \"-z\" output mode, 2026-09-26)\n\nThis causes \"git fast-import --\", \"git patch-id --\" and\n\"git diff-pairs --\" to offer no options, even though they are\nlisted by \"--git-completion-helper\".\n\nRemove the flag from these four builtins, like 66fa6e8ed8 (git.c:\nupdate NO_PARSEOPT markings, 2022-08-19) did for others.\n\nSigned-off-by: Tanga Joris \n---\n git.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/git.c b/git.c\nindex d7b5f88e52..2d27ad4175 100644\n--- a/git.c\n+++ b/git.c\n@@ -571,11 +571,11 @@ static struct cmd_struct commands[] = {\n \t{ \"diff\", cmd_diff, NO_PARSEOPT },\n \t{ \"diff-files\", cmd_diff_files, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT },\n \t{ \"diff-index\", cmd_diff_index, RUN_SETUP | NO_PARSEOPT },\n-\t{ \"diff-pairs\", cmd_diff_pairs, RUN_SETUP | NO_PARSEOPT },\n+\t{ \"diff-pairs\", cmd_diff_pairs, RUN_SETUP },\n \t{ \"diff-tree\", cmd_diff_tree, RUN_SETUP | NO_PARSEOPT },\n \t{ \"difftool\", cmd_difftool, RUN_SETUP_GENTLY },\n \t{ \"fast-export\", cmd_fast_export, RUN_SETUP },\n-\t{ \"fast-import\", cmd_fast_import, RUN_SETUP | NO_PARSEOPT },\n+\t{ \"fast-import\", cmd_fast_import, RUN_SETUP },\n \t{ \"fetch\", cmd_fetch, RUN_SETUP },\n \t{ \"fetch-pack\", cmd_fetch_pack, RUN_SETUP | NO_PARSEOPT },\n \t{ \"fmt-merge-msg\", cmd_fmt_merge_msg, RUN_SETUP },\n@@ -626,7 +626,7 @@ static struct cmd_struct commands[] = {\n \t{ \"pack-redundant\", cmd_pack_redundant, RUN_SETUP | NO_PARSEOPT | DEPRECATED },\n #endif\n \t{ \"pack-refs\", cmd_pack_refs, RUN_SETUP },\n-\t{ \"patch-id\", cmd_patch_id, RUN_SETUP_GENTLY | NO_PARSEOPT },\n+\t{ \"patch-id\", cmd_patch_id, RUN_SETUP_GENTLY },\n \t{ \"pickaxe\", cmd_blame, RUN_SETUP },\n \t{ \"prune\", cmd_prune, RUN_SETUP },\n \t{ \"prune-packed\", cmd_prune_packed, RUN_SETUP },\n@@ -676,7 +676,7 @@ static struct cmd_struct commands[] = {\n \t{ \"upload-archive--writer\", cmd_upload_archive_writer, NO_PARSEOPT },\n \t{ \"upload-pack\", cmd_upload_pack },\n \t{ \"url-parse\", cmd_url_parse },\n-\t{ \"var\", cmd_var, RUN_SETUP_GENTLY | NO_PARSEOPT },\n+\t{ \"var\", cmd_var, RUN_SETUP_GENTLY },\n \t{ \"verify-commit\", cmd_verify_commit, RUN_SETUP },\n \t{ \"verify-pack\", cmd_verify_pack },\n \t{ \"verify-tag\", cmd_verify_tag, RUN_SETUP },\n-- \n2.43.0\n\n\n"},{"id":"556870","messageId":"123de4e4-3387-4eef-8a05-9ed1ad1fce30@gmail.com","threadId":"66649","inReplyTo":"20261029231621.48958-1-tangajoris@gmail.com","subject":"Re: [Outreachy PATCH v3 1/2] parse-options: answer --git-completion-helper before command runs","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2026-10-30T09:44:45Z","receivedAt":"2026-10-30T09:44:45Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"Hi Tanga Joris,\n\nJust a gentle reminder about this portion of the MyFirstContribution[1]:\n\n It’s good manners to reply to each comment in the mailing list\n discussion instead of letting the next version of your patch be your\n only response. Tell the reviewer whether you plan to make the\n suggested change, keep the original, or pursue a different approach.\n This way reviewers can respond to your reasoning before you spend time\n preparing a version they may not agree with, and later do not need to\n inspect your v2 to figure out whether you implemented their comment or\n not.\n\nOn 10/30/26 04:46, Tanga Joris wrote:\n> fast-import parses its options only after reading the \"feature\"\n> lines from its input, and it creates a temporary pack file before\n> that. So \"git fast-import --git-completion-helper\" waits for input,\n> and each run leaves a tmp_pack_* file behind.\n> > bash completion runs the helper with the terminal as input, so\n> \"git fast-import --\" would freeze once fast-import is no\n> longer marked with the NO_PARSEOPT flag.\n> 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.\n\n> Since show_usage_with_options_if_asked() already answers -h and\n> --help-all before a command does any work, it now also answers\n> the completion helper, so fast-import and other commands that\n> call it reply without reading input.\n> > Reported-by: René Scharfe \n> Helped-by: Junio C Hamano \n> Signed-off-by: Tanga Joris \n> ---\n> Changes since v2:\n> - Moved the check from fast-import into\n> show_usage_with_options_if_asked() in parse-options,\n> as Junio suggested.\n> - 2/2 is unchanged\n> > parse-options.c | 6 ++++++\n> t/t9300-fast-import.sh | 8 ++++++++\n> 2 files changed, 14 insertions(+)\n> > diff --git a/parse-options.c b/parse-options.c\n> index 283a0f1dc2..f2a23b79a4 100644\n> --- a/parse-options.c\n> +++ b/parse-options.c\n> @@ -1536,6 +1536,12 @@ void show_usage_with_options_if_asked(int ac, const char **av,\n> \t\t\tusage_with_options_internal(NULL, usagestr, opts,\n> \t\t\t\t\t\t USAGE_FULL, USAGE_TO_STDOUT);\n> \t\t\texit(0);\n> +\t\t} else if (!strcmp(av[1], \"--git-completion-helper\")) {\n> +\t\t\tshow_gitcomp(opts, 0);\n> +\t\t\texit(0);\n> +\t\t} else if (!strcmp(av[1], \"--git-completion-helper-all\")) {\n> +\t\t\tshow_gitcomp(opts, 1);\n> +\t\t\texit(0);\n> \t\t}\n> \t}\n> }\n\nThe 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?\n\n[ References ]\n\n[1]: https://git-scm.com/docs/MyFirstContribution#reviewing\n\n-- \nSivaraam\n\n\n"},{"id":"556881","messageId":"d026d6a0-5b1b-4bbf-bd1f-5f41c9c2a4e4@gmail.com","threadId":"66649","inReplyTo":"20261029231621.48958-2-tangajoris@gmail.com","subject":"Re: [Outreachy PATCH v3 2/2] git.c: drop NO_PARSEOPT from builtins that use parse-options","fromName":"Kaartic Sivaraam","fromEmail":"kaartic.sivaraam@gmail.com","sentAt":"2026-10-30T10:21:57Z","receivedAt":"2026-10-30T10:21:57Z","isPatch":true,"sender":{"key":"kaartic.sivaraam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12448084?v=4"},"body":"On 10/30/26 04:46, Tanga Joris wrote:\n> diff --git a/git.c b/git.c\n> index d7b5f88e52..2d27ad4175 100644\n> --- a/git.c\n> +++ b/git.c\n> @@ -571,11 +571,11 @@ static struct cmd_struct commands[] = {\n> \t{ \"diff\", cmd_diff, NO_PARSEOPT },\n> \t{ \"diff-files\", cmd_diff_files, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT },\n> \t{ \"diff-index\", cmd_diff_index, RUN_SETUP | NO_PARSEOPT },\n> -\t{ \"diff-pairs\", cmd_diff_pairs, RUN_SETUP | NO_PARSEOPT },\n> +\t{ \"diff-pairs\", cmd_diff_pairs, RUN_SETUP },\n> \t{ \"diff-tree\", cmd_diff_tree, RUN_SETUP | NO_PARSEOPT },\n> \t{ \"difftool\", cmd_difftool, RUN_SETUP_GENTLY },\n> \t{ \"fast-export\", cmd_fast_export, RUN_SETUP },\n> -\t{ \"fast-import\", cmd_fast_import, RUN_SETUP | NO_PARSEOPT },\n> +\t{ \"fast-import\", cmd_fast_import, RUN_SETUP },\n> \t{ \"fetch\", cmd_fetch, RUN_SETUP },\n> \t{ \"fetch-pack\", cmd_fetch_pack, RUN_SETUP | NO_PARSEOPT },\n> \t{ \"fmt-merge-msg\", cmd_fmt_merge_msg, RUN_SETUP },\n> @@ -626,7 +626,7 @@ static struct cmd_struct commands[] = {\n> \t{ \"pack-redundant\", cmd_pack_redundant, RUN_SETUP | NO_PARSEOPT | DEPRECATED },\n> #endif\n> \t{ \"pack-refs\", cmd_pack_refs, RUN_SETUP },\n> -\t{ \"patch-id\", cmd_patch_id, RUN_SETUP_GENTLY | NO_PARSEOPT },\n> +\t{ \"patch-id\", cmd_patch_id, RUN_SETUP_GENTLY },\n> \t{ \"pickaxe\", cmd_blame, RUN_SETUP },\n> \t{ \"prune\", cmd_prune, RUN_SETUP },\n> \t{ \"prune-packed\", cmd_prune_packed, RUN_SETUP },\n> @@ -676,7 +676,7 @@ static struct cmd_struct commands[] = {\n> \t{ \"upload-archive--writer\", cmd_upload_archive_writer, NO_PARSEOPT },\n> \t{ \"upload-pack\", cmd_upload_pack },\n> \t{ \"url-parse\", cmd_url_parse },\n> -\t{ \"var\", cmd_var, RUN_SETUP_GENTLY | NO_PARSEOPT },\n> +\t{ \"var\", cmd_var, RUN_SETUP_GENTLY },\n> \t{ \"verify-commit\", cmd_verify_commit, RUN_SETUP },\n> \t{ \"verify-pack\", cmd_verify_pack },\n> \t{ \"verify-tag\", cmd_verify_tag, RUN_SETUP },\n\nThis 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?\n\nFor instance, something like so:\n '\ndiff --git a/t/t0012-help.sh b/t/t0012-help.sh\nindex b0a3fdbbbc..cb147f30ef 100755\n--- a/t/t0012-help.sh\n+++ b/t/t0012-help.sh\n@@ -267,4 +267,29 @@ do\n \t'\n done parseopt &&\n+\tsort builtins >sorted-builtins &&\n+\tsort parseopt >sorted-parseopt &&\n+\tfalse &&\n+\tcomm -23 sorted-builtins sorted-parseopt >no-parseopt\n+'\n+\n+while read builtin\n+do\n+\tcase \"$builtin\" in\n+\t# these parse their own options by hand, but pass the diff\n+\t# options through parse-options, which answers the helper\n+\tdiff|diff-files|diff-index|diff-tree|rev-list)\n+\t\tcontinue ;;\n+\tesac\n+\n+\ttest_expect_success \"$builtin is not using parse-options\" '\n+\t\ttest_might_fail git $builtin --git-completion-helper \\\n+\t\t\toutput 2>/dev/null &&\n+\t\t# rev-parse echoes unknown arguments back\n+\t\t! grep -v \"^--git-completion-helper\\$\" output | grep \"^--\"\n+\t'\n+done wrote:\n> Just a gentle reminder about this portion of the MyFirstContribution[1]:\n>\n> It’s good manners to reply to each comment in the mailing list\n> discussion instead of letting the next version of your patch be your\n> only response. Tell the reviewer whether you plan to make the\n> suggested change, keep the original, or pursue a different approach.\n\nThanks for the reminder about replying to comments. I'll do that from\nnow on.\n\n> May be give a hint that you plan to do this in the following commit?\n> That might give someone without context (for example someone reading the\n> commit after a while) a hint about why you are worrying about marking\n> fast-import with the NO_PARSEOPT flag.\n\nAlright, I'll add a sentence to the commit message saying that the\nnext commit removes the NO_PARSEOPT flag from fast-import, which would\nmake \"git fast-import --\" freeze without this patch.\n\n> The change seems to be fine but I wonder if it is worth a note in\n> parse-options.h about why show_usage_with_options_if_asked() is also\n> handling the '--git-completion-helper' and '--git-completion-helper-all'\n> arguments?\n\nYes, I think that would help, so I will add a short comment above its\ndeclaration in parse-options.h explaining that it answers these before\nthe command does any work.\n\nThanks,\nTanga\n\n"},{"id":"557111","messageId":"CAKrRMNogw6rUsZzDsEPLmLAmjB6fvcH4njga+_qBz554x8oEwg@mail.gmail.com","threadId":"66649","inReplyTo":"d026d6a0-5b1b-4bbf-bd1f-5f41c9c2a4e4@gmail.com","subject":"Re: [Outreachy PATCH v3 2/2] git.c: drop NO_PARSEOPT from builtins that use parse-options","fromName":"Tanga Joris","fromEmail":"tangajoris@gmail.com","sentAt":"2026-11-02T02:26:47Z","receivedAt":"2026-11-02T02:26:47Z","isPatch":true,"sender":{"key":"tangajoris@gmail.com","avatar":null},"body":"Hello Kaartic,\n\nThanks for reviewing my patch 2/2, and for writing the test sketch.\n\nOn Fri, 30 Oct 2026 at 13:21, Kaartic Sivaraam\n wrote:\n> This looks good to me. I was wondering if it would be valuable to also\n> add a test so that any future commands that start using parse_options()\n> could be caught if the corresponding change does not also remove\n> NO_PARSEOPT from the command?\n\nI also think the test is a good idea. It would definitely catch future commands\nthat switch to parse-options but forget to remove NO_PARSEOPT.\n\nI tried your sketch on top of this series. Without 2/2, it fails for\ndiff-pairs, fast-import and patch-id, so it catches the bug this\npatch fixes. With patch 2/2, it passes. I had to remove the\n'false &&' line to make it run, which I think was left over from testing.\n\nIt does not catch var, though, because var only has short options\n(like -l), and the test looks for lines starting with '--'.\n\nCurrently, I am not sure how to make the test catch commands with only short\noptions, because the helper only lists those with long options. Do you\nhave an idea on how we could do this?\n\nWould you like the test inside 2/2, or as a separate patch 3/3?\n\nThanks,\nTanga\n\n"},{"id":"557280","messageId":"20261103092503.14872-1-herroyalpriestess@gmail.com","threadId":"66649","inReplyTo":"20261029231621.48958-1-tangajoris@gmail.com","subject":"Re: [Outreachy PATCH v3 1/2] parse-options: answer --git-completion-helper before command runs","fromName":"Cynthia Uche","fromEmail":"herroyalpriestess@gmail.com","sentAt":"2026-11-03T09:25:01Z","receivedAt":"2026-11-03T09:25:01Z","isPatch":true,"sender":{"key":"herroyalpriestess@gmail.com","avatar":null},"body":"Hi Tanga,\n\nI applied v3 1/2 and 2/2 on top of master (ebdc087dc9) and built with\nNO_RUST=1. t9300, t0012 and t9902 pass. On master,\n\"git --list-cmds=parseopt\" lists none of diff-pairs, fast-import,\npatch-id and var; with the series it lists all four, and the first\nthree print their options with --git-completion-helper.\n\nOn Fri, Oct 30, 2026 at 12:17 AM Tanga Joris wrote:\n> +\t\t} else if (!strcmp(av[1], \"--git-completion-helper-all\")) {\n> +\t\t\tshow_gitcomp(opts, 1);\n> +\t\t\texit(0);\n\nSince this affects every caller of show_usage_with_options_if_asked(),\nI ran \"git --git-completion-helper\" and the \"-all\" form with\nall &&\n \ttest_grep -e \"--allow-unsafe-features\" all\n '\n\nCynthia\n\n"},{"id":"557508","messageId":"CAKrRMNpaF=NQOi0mmr_1jE0TQynbQRzhEtdSj2RLfon-UHMLgw@mail.gmail.com","threadId":"66649","inReplyTo":"20261103092503.14872-1-herroyalpriestess@gmail.com","subject":"Re: [Outreachy PATCH v3 1/2] parse-options: answer --git-completion-helper before command runs","fromName":"Tanga Joris","fromEmail":"tangajoris@gmail.com","sentAt":"2026-11-04T22:34:23Z","receivedAt":"2026-11-04T22:34:23Z","isPatch":true,"sender":{"key":"tangajoris@gmail.com","avatar":null},"body":"Hey Cynthia,\n\nThank you for testing this.\n\nOn Tue, 3 Nov 2026 at 12:25, Cynthia Uche wrote:\n> The new t9300 test fails without the parse-options.c change, so it\n> guards the fix, but it only covers the plain form: with the \"-all\"\n> branch changed to show_gitcomp(opts, 0) it still passes. Could you\n> add a test for that branch?\n\nYes, I will add the test in my v4. I have tried it locally myself and\nit passes with my fix\nand fails when the -all branch is broken. Thanks again, I will credit\nyou with Helped-by:.\n\nTanga\n\n"},{"id":"557522","messageId":"20261105003914.708836-1-tangajoris@gmail.com","threadId":"66649","inReplyTo":"20261029231621.48958-1-tangajoris@gmail.com","subject":"[Outreachy PATCH v4 1/3] parse-options: answer --git-completion-helper before command runs","fromName":"Tanga Joris","fromEmail":"tangajoris@gmail.com","sentAt":"2026-11-05T00:39:11Z","receivedAt":"2026-11-05T00:39:11Z","isPatch":true,"sender":{"key":"tangajoris@gmail.com","avatar":null},"body":"fast-import parses its options only after reading the \"feature\"\nlines from its input, and it creates a temporary pack file before\nthat. So \"git fast-import --git-completion-helper\" waits for input,\nand each run leaves a tmp_pack_* file behind.\n\nbash completion runs the helper with the terminal as input. The next\ncommit removes the NO_PARSEOPT flag from fast-import, which would make\n\"git fast-import --\" freeze without this patch.\n\nshow_usage_with_options_if_asked() already answers -h and --help-all\nbefore a command does any work. Teach it to answer the completion\nhelper too. Then fast-import, and the other commands that call it,\nreply without reading input.\n\nReported-by: René Scharfe \nHelped-by: Junio C Hamano \nHelped-by: Kaartic Sivaraam \nHelped-by: Cynthia Uche \nSigned-off-by: Tanga Joris \n---\nChanges since v3:\n - Added a sentence to the log message saying that the next commit\n removes NO_PARSEOPT from fast-import, as Kaartic suggested.\n - Added a comment above show_usage_with_options_if_asked() in\n parse-options.h, as Kaartic suggested.\n - Added Cynthia's test for --git-completion-helper-all. I also added\n the tmp_pack_* check from the first test to it, because without\n it the test still passes when the fix is reverted.\n - 2/3 is unchanged.\n - 3/3 is new, it adds the test Kaartic suggested.\n\n parse-options.c | 6 ++++++\n parse-options.h | 6 ++++++\n t/t9300-fast-import.sh | 16 ++++++++++++++++\n 3 files changed, 28 insertions(+)\n\ndiff --git a/parse-options.c b/parse-options.c\nindex 283a0f1dc2..f2a23b79a4 100644\n--- a/parse-options.c\n+++ b/parse-options.c\n@@ -1536,6 +1536,12 @@ void show_usage_with_options_if_asked(int ac, const char **av,\n \t\t\tusage_with_options_internal(NULL, usagestr, opts,\n \t\t\t\t\t\t USAGE_FULL, USAGE_TO_STDOUT);\n \t\t\texit(0);\n+\t\t} else if (!strcmp(av[1], \"--git-completion-helper\")) {\n+\t\t\tshow_gitcomp(opts, 0);\n+\t\t\texit(0);\n+\t\t} else if (!strcmp(av[1], \"--git-completion-helper-all\")) {\n+\t\t\tshow_gitcomp(opts, 1);\n+\t\t\texit(0);\n \t\t}\n \t}\n }\ndiff --git a/parse-options.h b/parse-options.h\nindex f68b51ceca..7ad33801fe 100644\n--- a/parse-options.h\n+++ b/parse-options.h\n@@ -427,6 +427,12 @@ int parse_options(int argc, const char **argv, const char *prefix,\n NORETURN void usage_with_options(const char * const *usagestr,\n \t\t\t\t const struct option *options);\n \n+/*\n+ * If the only argument is -h, --help-all, --git-completion-helper or\n+ * --git-completion-helper-all, print the answer and exit. Call this\n+ * before the command does any work. Then asking for help or for the\n+ * completion list does not wait for input or leave files behind.\n+ */\n void show_usage_with_options_if_asked(int ac, const char **av,\n \t\t\t\t const char * const *usage,\n \t\t\t\t const struct option *options);\ndiff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh\nindex 73c6e3d448..1305614381 100755\n--- a/t/t9300-fast-import.sh\n+++ b/t/t9300-fast-import.sh\n@@ -2914,6 +2914,22 @@ test_expect_success 'R: blob appears only once' '\n \ttest 1 = $n\n '\n \n+test_expect_success 'R: --git-completion-helper does not read from stdin' '\n+\tfind .git/objects/pack -name \"tmp_pack_*\" >before &&\n+\techo garbage | git fast-import --git-completion-helper >out &&\n+\ttest_grep -e \"--date-format=\" out &&\n+\tfind .git/objects/pack -name \"tmp_pack_*\" >after &&\n+\ttest_cmp before after\n+'\n+\n+test_expect_success 'R: --git-completion-helper-all does not read from stdin' '\n+\tfind .git/objects/pack -name \"tmp_pack_*\" >before &&\n+\techo garbage | git fast-import --git-completion-helper-all >all &&\n+\ttest_grep -e \"--allow-unsafe-features\" all &&\n+\tfind .git/objects/pack -name \"tmp_pack_*\" >after &&\n+\ttest_cmp before after\n+'\n+\n ###\n ### series S (mark and path parsing)\n ###\n-- \n2.43.0\n\n\n"},{"id":"557523","messageId":"20261105003914.708836-2-tangajoris@gmail.com","threadId":"66649","inReplyTo":"20261105003914.708836-1-tangajoris@gmail.com","subject":"[Outreachy PATCH v4 2/3] git.c: drop NO_PARSEOPT from builtins that use parse-options","fromName":"Tanga Joris","fromEmail":"tangajoris@gmail.com","sentAt":"2026-11-05T00:39:12Z","receivedAt":"2026-11-05T00:39:12Z","isPatch":true,"sender":{"key":"tangajoris@gmail.com","avatar":null},"body":"The NO_PARSEOPT flag in the commands[] table marks builtins that\nparse their options by hand. Builtins marked with it are left out\nof \"git --list-cmds=parseopt\", which bash completion uses to\ndecide whether it can ask a command for its options with\n\"--git-completion-helper\".\n\nFour builtins have since been converted to parse-options, but are\nstill marked NO_PARSEOPT:\n\n - patch-id, in 2871f4d447 (builtin: patch-id: add --verbatim as a\n command mode, 2022-10-24)\n - diff-pairs, which has used parse-options since it was introduced\n in 5bd10b2adc (builtin: introduce diff-pairs command, 2025-02-28)\n - fast-import, in 863937696e (fast-import: use parse_options() for\n command line options, 2026-08-11)\n - var, in e19f0779c6 (var: add \"-z\" output mode, 2026-09-26)\n\nThis causes \"git fast-import --\", \"git patch-id --\" and\n\"git diff-pairs --\" to offer no options, even though they are\nlisted by \"--git-completion-helper\".\n\nRemove the flag from these four builtins, like 66fa6e8ed8 (git.c:\nupdate NO_PARSEOPT markings, 2022-08-19) did for others.\n\nSigned-off-by: Tanga Joris \n---\n git.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/git.c b/git.c\nindex d7b5f88e52..2d27ad4175 100644\n--- a/git.c\n+++ b/git.c\n@@ -571,11 +571,11 @@ static struct cmd_struct commands[] = {\n \t{ \"diff\", cmd_diff, NO_PARSEOPT },\n \t{ \"diff-files\", cmd_diff_files, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT },\n \t{ \"diff-index\", cmd_diff_index, RUN_SETUP | NO_PARSEOPT },\n-\t{ \"diff-pairs\", cmd_diff_pairs, RUN_SETUP | NO_PARSEOPT },\n+\t{ \"diff-pairs\", cmd_diff_pairs, RUN_SETUP },\n \t{ \"diff-tree\", cmd_diff_tree, RUN_SETUP | NO_PARSEOPT },\n \t{ \"difftool\", cmd_difftool, RUN_SETUP_GENTLY },\n \t{ \"fast-export\", cmd_fast_export, RUN_SETUP },\n-\t{ \"fast-import\", cmd_fast_import, RUN_SETUP | NO_PARSEOPT },\n+\t{ \"fast-import\", cmd_fast_import, RUN_SETUP },\n \t{ \"fetch\", cmd_fetch, RUN_SETUP },\n \t{ \"fetch-pack\", cmd_fetch_pack, RUN_SETUP | NO_PARSEOPT },\n \t{ \"fmt-merge-msg\", cmd_fmt_merge_msg, RUN_SETUP },\n@@ -626,7 +626,7 @@ static struct cmd_struct commands[] = {\n \t{ \"pack-redundant\", cmd_pack_redundant, RUN_SETUP | NO_PARSEOPT | DEPRECATED },\n #endif\n \t{ \"pack-refs\", cmd_pack_refs, RUN_SETUP },\n-\t{ \"patch-id\", cmd_patch_id, RUN_SETUP_GENTLY | NO_PARSEOPT },\n+\t{ \"patch-id\", cmd_patch_id, RUN_SETUP_GENTLY },\n \t{ \"pickaxe\", cmd_blame, RUN_SETUP },\n \t{ \"prune\", cmd_prune, RUN_SETUP },\n \t{ \"prune-packed\", cmd_prune_packed, RUN_SETUP },\n@@ -676,7 +676,7 @@ static struct cmd_struct commands[] = {\n \t{ \"upload-archive--writer\", cmd_upload_archive_writer, NO_PARSEOPT },\n \t{ \"upload-pack\", cmd_upload_pack },\n \t{ \"url-parse\", cmd_url_parse },\n-\t{ \"var\", cmd_var, RUN_SETUP_GENTLY | NO_PARSEOPT },\n+\t{ \"var\", cmd_var, RUN_SETUP_GENTLY },\n \t{ \"verify-commit\", cmd_verify_commit, RUN_SETUP },\n \t{ \"verify-pack\", cmd_verify_pack },\n \t{ \"verify-tag\", cmd_verify_tag, RUN_SETUP },\n-- \n2.43.0\n\n\n"},{"id":"557524","messageId":"20261105003914.708836-3-tangajoris@gmail.com","threadId":"66649","inReplyTo":"20261105003914.708836-1-tangajoris@gmail.com","subject":"[Outreachy PATCH v4 3/3] t0012: catch builtins that keep NO_PARSEOPT after conversion","fromName":"Tanga Joris","fromEmail":"tangajoris@gmail.com","sentAt":"2026-11-05T00:39:13Z","receivedAt":"2026-11-05T00:39:13Z","isPatch":true,"sender":{"key":"tangajoris@gmail.com","avatar":null},"body":"When a builtin is converted to parse-options, the NO_PARSEOPT flag in\ngit.c has to be removed by hand. No test checks this. The previous\ncommit removes the flag from four builtins where it was left behind.\n\nAdd a test that runs \"git --git-completion-helper\" for every\nbuiltin left out of \"git --list-cmds=parseopt\". The test fails if the\ncommand prints an option starting with \"--\". Without the previous\ncommit, it fails for diff-pairs, fast-import and patch-id.\n\nThe test skips diff, diff-files, diff-index, diff-tree and rev-list,\nbecause they print the diff options. It also skips rev-parse, because\nit prints the argument back. It does not catch var, because var only\nhas short options (like -l) and the helper only lists long options.\n\nSuggested-by: Kaartic Sivaraam \nSigned-off-by: Tanga Joris \n---\nKaartic, I put your test in a separate patch for now, so it is easy to\nfind. I'm happy to squash it into 2/3 if you prefer it there.\n\n t/t0012-help.sh | 25 +++++++++++++++++++++++++\n 1 file changed, 25 insertions(+)\n\ndiff --git a/t/t0012-help.sh b/t/t0012-help.sh\nindex b0a3fdbbbc..41197c67e2 100755\n--- a/t/t0012-help.sh\n+++ b/t/t0012-help.sh\n@@ -267,4 +267,29 @@ do\n \t'\n done parseopt &&\n+\ttr \" \" \"\\n\" sorted-parseopt &&\n+\tsort builtins >sorted-builtins &&\n+\tcomm -23 sorted-builtins sorted-parseopt >no-parseopt\n+'\n+\n+while read builtin\n+do\n+\tcase \"$builtin\" in\n+\t# these answer with the diff options\n+\tdiff|diff-files|diff-index|diff-tree|rev-list)\n+\t\tcontinue ;;\n+\t# rev-parse prints unknown arguments back\n+\trev-parse)\n+\t\tcontinue ;;\n+\tesac\n+\n+\ttest_expect_success \"$builtin does not answer --git-completion-helper\" '\n+\t\ttest_might_fail git $builtin --git-completion-helper \\\n+\t\t\toutput 2>/dev/null &&\n+\t\ttest_grep ! \"^--\" output\n+\t'\n+done