{"thread":{"id":"66132","subject":"[PATCH v2 0/4] completion: add support for 'git history'","startedAt":"2026-08-06T20:27:46Z","lastAt":"2026-08-13T13:45:15Z","messageCount":17,"participants":["Vincent Mailhol","Patrick Steinhardt","D. Ben Knoble","Ben Knoble"],"isPatch":true,"patchVersion":2,"patchTotal":4},"messages":[{"id":"549884","messageId":"20260806-history_autocompletion-v2-0-7e60f52a1c20@kernel.org","threadId":"66132","inReplyTo":null,"subject":"[PATCH v2 0/4] completion: add support for 'git history'","fromName":"Vincent Mailhol","fromEmail":"mailhol@kernel.org","sentAt":"2026-08-06T20:27:35Z","receivedAt":"2026-08-06T20:27:46Z","isPatch":true,"body":"This series adds Bash completion for the subcommands of \"git history\"\nand their options.\n\nPatch #1 adds the basic subcommand and options completion. Patch #2\nand #3 take care of the value of the --empty and --update-refs options.\nFinally, Patch #4 adds completion for pathspecs accepted by \"split\".\n\nFor each of the completions, add a set of relevant test cases.\n\nSigned-off-by: Vincent Mailhol <mailhol@kernel.org>\n---\nChanges in v2:\n\n  - Complete exactly one required revision and leave subsequent\n    arguments to subcommand-specific completion.\n  - Do not complete options after \"--\".\n  - Complete values for \"--empty\" and \"--update-refs\".\n  - Complete pathspecs for \"git history split\".\n  - Expand the test coverage for options, revisions, and pathspecs.\n\nLink to v1: https://lore.kernel.org/r/20260804-history_autocompletion-v1-1-6f7459ffb677@kernel.org\n\n---\nVincent Mailhol (4):\n      completion: add 'git history' subcommands\n      completion: complete 'git history --empty' values\n      completion: complete 'git history --update-refs' values\n      completion: complete 'git history split' pathspecs\n\n contrib/completion/git-completion.bash | 68 ++++++++++++++++++++++++++++++++++\n t/t9902-completion.sh                  | 49 ++++++++++++++++++++++++\n 2 files changed, 117 insertions(+)\n\nRange-diff versus v1:\n\n1:  d0574dca8c ! 1:  6625c7ac29 completion: add 'git history' subcommands\n    @@ Metadata\n      ## Commit message ##\n         completion: add 'git history' subcommands\n     \n    -    Use the parse-options completion helpers for the \"git history\"\n    -    subcommands and their options. Complete positional arguments as\n    -    revisions, and add coverage for each kind of completion.\n    +    Use the parse-options completion helpers for the\n    +\n    +      git history\n    +\n    +    subcommands and their options. All current history subcommands take a\n    +    revision as their first positional argument, so complete that argument\n    +    as a revision.\n    +\n    +    Once the revision is present, leave any further positional arguments to\n    +    subcommand-specific completion. This allows a subcommand to complete\n    +    another kind of argument, such as the pathspec accepted by\n    +\n    +      git history split\n    +\n    +    or another revision if a future subcommand accepts one.\n     \n         Signed-off-by: Vincent Mailhol <mailhol@kernel.org>\n    +    ---\n    +    Changes in v2:\n    +\n    +      - Test options before and after revisions.\n    +      - Do not complete options after \"--\".\n    +      - Stop revision completion after the first required\n     \n      ## contrib/completion/git-completion.bash ##\n     @@ contrib/completion/git-completion.bash: _git_help ()\n      \tfi\n      }\n      \n    ++__git_history_has_revision ()\n    ++{\n    ++\tlocal i\n    ++\n    ++\tfor ((i = __git_cmd_idx + 2; i < cword; i++)); do\n    ++\t\tcase \"${words[i]}\" in\n    ++\t\t--empty|--update-refs)\n    ++\t\t\t((i++))\n    ++\t\t\t;;\n    ++\t\t-*)\n    ++\t\t\t;;\n    ++\t\t*)\n    ++\t\t\treturn 0\n    ++\t\t\t;;\n    ++\t\tesac\n    ++\tdone\n    ++\treturn 1\n    ++}\n    ++\n     +_git_history ()\n     +{\n     +\tlocal subcommands subcommand\n    @@ contrib/completion/git-completion.bash: _git_help ()\n     +\t\treturn\n     +\tfi\n     +\n    -+\tcase \"$cur\" in\n    -+\t--*)\n    -+\t\t__gitcomp_builtin \"history_$subcommand\"\n    -+\t\t;;\n    -+\t*)\n    ++\tif ! __git_has_doubledash; then\n    ++\t\tcase \"$cur\" in\n    ++\t\t--*)\n    ++\t\t\t__gitcomp_builtin \"history_$subcommand\"\n    ++\t\t\treturn\n    ++\t\t\t;;\n    ++\t\tesac\n    ++\tfi\n    ++\n    ++\tif ! __git_history_has_revision; then\n     +\t\t__git_complete_refs\n    -+\t\t;;\n    -+\tesac\n    ++\t\treturn\n    ++\tfi\n     +}\n     +\n      _git_init ()\n    @@ t/t9902-completion.sh: test_expect_success 'git clone --config= - value' '\n     +'\n     +\n     +test_expect_success 'git history subcommand options' '\n    -+\ttest_completion \"git history fixup --upd\" \"--update-refs=\"\n    ++\ttest_completion \"git history split main --\" <<-\\EOF &&\n    ++\t--update-refs=Z\n    ++\t--dry-run Z\n    ++\t--no-dry-run Z\n    ++\tEOF\n    ++\ttest_completion \"git history fixup --upd\" \"--update-refs=\" &&\n    ++\ttest_completion \"git history fixup --ree\" \"--reedit-message \" &&\n    ++\ttest_completion \"git history split --upd\" \"--update-refs=\" &&\n    ++\ttest_completion \"git history split main --dry\" \"--dry-run \" &&\n    ++\ttest_completion \"git history reword main -- --d\" \"\"\n     +'\n     +\n     +test_expect_success 'git history revisions' '\n    -+\ttest_completion \"git history split ma\" \"main \"\n    ++\ttest_completion \"git history split ma\" \"main \" &&\n    ++\ttest_completion \"git history split --update-refs head ma\" \"main \" &&\n    ++\ttest_completion \"git history fixup --empty drop ma\" \"main \" &&\n    ++\ttest_completion \"git history reword main m\" \"\"\n     +'\n     +\n      test_expect_success 'git reflog show' '\n-:  ---------- > 2:  f618f35153 completion: complete 'git history --empty' values\n-:  ---------- > 3:  abae09f208 completion: complete 'git history --update-refs' values\n-:  ---------- > 4:  7bfb6664dc completion: complete 'git history split' pathspecs\n\n---\nbase-commit: c56d675cccfbcf71406c4a6806c7745e4a756294\nchange-id: 20260804-history_autocompletion-84620c2f8500\n\n"},{"id":"549885","messageId":"20260806-history_autocompletion-v2-1-7e60f52a1c20@kernel.org","threadId":"66132","inReplyTo":"20260806-history_autocompletion-v2-0-7e60f52a1c20@kernel.org","subject":"[PATCH v2 1/4] completion: add 'git history' subcommands","fromName":"Vincent Mailhol","fromEmail":"mailhol@kernel.org","sentAt":"2026-08-06T20:27:36Z","receivedAt":"2026-08-06T20:27:48Z","isPatch":true,"body":"Use the parse-options completion helpers for the\n\n  git history\n\nsubcommands and their options. All current history subcommands take a\nrevision as their first positional argument, so complete that argument\nas a revision.\n\nOnce the revision is present, leave any further positional arguments to\nsubcommand-specific completion. This allows a subcommand to complete\nanother kind of argument, such as the pathspec accepted by\n\n  git history split\n\nor another revision if a future subcommand accepts one.\n\nSigned-off-by: Vincent Mailhol <mailhol@kernel.org>\n---\nChanges in v2:\n\n  - Test options before and after revisions.\n  - Do not complete options after \"--\".\n  - Stop revision completion after the first required\n---\n contrib/completion/git-completion.bash | 48 ++++++++++++++++++++++++++++++++++\n t/t9902-completion.sh                  | 29 ++++++++++++++++++++\n 2 files changed, 77 insertions(+)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex e875787710..7372e2919b 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -2137,6 +2137,54 @@ _git_help ()\n \tfi\n }\n \n+__git_history_has_revision ()\n+{\n+\tlocal i\n+\n+\tfor ((i = __git_cmd_idx + 2; i < cword; i++)); do\n+\t\tcase \"${words[i]}\" in\n+\t\t--empty|--update-refs)\n+\t\t\t((i++))\n+\t\t\t;;\n+\t\t-*)\n+\t\t\t;;\n+\t\t*)\n+\t\t\treturn 0\n+\t\t\t;;\n+\t\tesac\n+\tdone\n+\treturn 1\n+}\n+\n+_git_history ()\n+{\n+\tlocal subcommands subcommand\n+\n+\t__git_resolve_builtins \"history\"\n+\n+\tsubcommands=\"$___git_resolved_builtins\"\n+\tsubcommand=\"$(__git_find_subcommand \"$subcommands\")\"\n+\n+\tif [ -z \"$subcommand\" ]; then\n+\t\t__gitcomp \"$subcommands\"\n+\t\treturn\n+\tfi\n+\n+\tif ! __git_has_doubledash; then\n+\t\tcase \"$cur\" in\n+\t\t--*)\n+\t\t\t__gitcomp_builtin \"history_$subcommand\"\n+\t\t\treturn\n+\t\t\t;;\n+\t\tesac\n+\tfi\n+\n+\tif ! __git_history_has_revision; then\n+\t\t__git_complete_refs\n+\t\treturn\n+\tfi\n+}\n+\n _git_init ()\n {\n \tcase \"$cur\" in\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 9ae3c48ebd..5ccb38c751 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -3107,6 +3107,35 @@ test_expect_success 'git clone --config= - value' '\n \tEOF\n '\n \n+test_expect_success 'git history subcommands' '\n+\ttest_completion \"git history \" <<-\\EOF\n+\tdrop Z\n+\tfixup Z\n+\treword Z\n+\tsplit Z\n+\tEOF\n+'\n+\n+test_expect_success 'git history subcommand options' '\n+\ttest_completion \"git history split main --\" <<-\\EOF &&\n+\t--update-refs=Z\n+\t--dry-run Z\n+\t--no-dry-run Z\n+\tEOF\n+\ttest_completion \"git history fixup --upd\" \"--update-refs=\" &&\n+\ttest_completion \"git history fixup --ree\" \"--reedit-message \" &&\n+\ttest_completion \"git history split --upd\" \"--update-refs=\" &&\n+\ttest_completion \"git history split main --dry\" \"--dry-run \" &&\n+\ttest_completion \"git history reword main -- --d\" \"\"\n+'\n+\n+test_expect_success 'git history revisions' '\n+\ttest_completion \"git history split ma\" \"main \" &&\n+\ttest_completion \"git history split --update-refs head ma\" \"main \" &&\n+\ttest_completion \"git history fixup --empty drop ma\" \"main \" &&\n+\ttest_completion \"git history reword main m\" \"\"\n+'\n+\n test_expect_success 'git reflog show' '\n \ttest_when_finished \"git checkout - && git branch -d shown\" &&\n \tgit checkout -b shown &&\n\n-- \n2.54.0\n\n"},{"id":"549886","messageId":"20260806-history_autocompletion-v2-2-7e60f52a1c20@kernel.org","threadId":"66132","inReplyTo":"20260806-history_autocompletion-v2-0-7e60f52a1c20@kernel.org","subject":"[PATCH v2 2/4] completion: complete 'git history --empty' values","fromName":"Vincent Mailhol","fromEmail":"mailhol@kernel.org","sentAt":"2026-08-06T20:27:37Z","receivedAt":"2026-08-06T20:27:50Z","isPatch":true,"body":"The \"--empty\" option accepts \"drop\", \"keep\", or \"abort\" for the \"drop\"\nand \"fixup\" subcommands. Complete these values.\n\nAlthough the synopsis only documents the:\n\n  --empty=<value>\n\nform, parse-options also accepts the value as a separate argument:\n\n  --empty <value>\n\nSupport both forms to follow the parser.\n\nSigned-off-by: Vincent Mailhol <mailhol@kernel.org>\n---\nChanges in v2:\n\n  - New patch.\n---\n contrib/completion/git-completion.bash | 13 +++++++++++--\n t/t9902-completion.sh                  |  5 ++++-\n 2 files changed, 15 insertions(+), 3 deletions(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 7372e2919b..fe5223b8ec 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -2171,8 +2171,17 @@ _git_history ()\n \tfi\n \n \tif ! __git_has_doubledash; then\n-\t\tcase \"$cur\" in\n-\t\t--*)\n+\t\tcase \"$prev,$cur\" in\n+\t\t--empty,*|*,--empty=*)\n+\t\t\tcase \"$subcommand\" in\n+\t\t\tdrop|fixup)\n+\t\t\t\t__gitcomp \"drop keep abort\" \"\" \\\n+\t\t\t\t\t\"${cur##--empty=}\"\n+\t\t\t\treturn\n+\t\t\t\t;;\n+\t\t\tesac\n+\t\t\t;;\n+\t\t*,--*)\n \t\t\t__gitcomp_builtin \"history_$subcommand\"\n \t\t\treturn\n \t\t\t;;\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 5ccb38c751..52a036a1ad 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -3126,7 +3126,10 @@ test_expect_success 'git history subcommand options' '\n \ttest_completion \"git history fixup --ree\" \"--reedit-message \" &&\n \ttest_completion \"git history split --upd\" \"--update-refs=\" &&\n \ttest_completion \"git history split main --dry\" \"--dry-run \" &&\n-\ttest_completion \"git history reword main -- --d\" \"\"\n+\ttest_completion \"git history reword main -- --d\" \"\" &&\n+\ttest_completion \"git history fixup --empty=ke\" \"keep \" &&\n+\ttest_completion \"git history drop --empty ab\" \"abort \" &&\n+\ttest_completion \"git history reword --empty=ke\" \"\"\n '\n \n test_expect_success 'git history revisions' '\n\n-- \n2.54.0\n\n"},{"id":"549887","messageId":"20260806-history_autocompletion-v2-3-7e60f52a1c20@kernel.org","threadId":"66132","inReplyTo":"20260806-history_autocompletion-v2-0-7e60f52a1c20@kernel.org","subject":"[PATCH v2 3/4] completion: complete 'git history --update-refs' values","fromName":"Vincent Mailhol","fromEmail":"mailhol@kernel.org","sentAt":"2026-08-06T20:27:38Z","receivedAt":"2026-08-06T20:27:52Z","isPatch":true,"body":"The \"--update-refs\" option accepts either \"branches\" or \"head\".\nComplete these values.\n\nAlthough the synopsis only documents the:\n\n  --update-refs=<value>\n\nform, parse-options also accepts the value as a separate argument:\n\n  --update-refs <value>\n\nSupport both forms to follow the parser.\n\nSigned-off-by: Vincent Mailhol <mailhol@kernel.org>\n---\nChanges in v2:\n\n  - New patch.\n---\n contrib/completion/git-completion.bash | 5 +++++\n t/t9902-completion.sh                  | 6 +++++-\n 2 files changed, 10 insertions(+), 1 deletion(-)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex fe5223b8ec..6f1ba96763 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -2181,6 +2181,11 @@ _git_history ()\n \t\t\t\t;;\n \t\t\tesac\n \t\t\t;;\n+\t\t--update-refs,*|*,--update-refs=*)\n+\t\t\t__gitcomp \"branches head\" \"\" \\\n+\t\t\t\t\"${cur##--update-refs=}\"\n+\t\t\treturn\n+\t\t\t;;\n \t\t*,--*)\n \t\t\t__gitcomp_builtin \"history_$subcommand\"\n \t\t\treturn\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex 52a036a1ad..ea86ecc08f 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -3129,7 +3129,11 @@ test_expect_success 'git history subcommand options' '\n \ttest_completion \"git history reword main -- --d\" \"\" &&\n \ttest_completion \"git history fixup --empty=ke\" \"keep \" &&\n \ttest_completion \"git history drop --empty ab\" \"abort \" &&\n-\ttest_completion \"git history reword --empty=ke\" \"\"\n+\ttest_completion \"git history reword --empty=ke\" \"\" &&\n+\ttest_completion \"git history fixup --update-refs=he\" \"head \" &&\n+\ttest_completion \"git history split --update-refs he\" \"head \" &&\n+\ttest_completion \"git history reword main -- --update-refs=he\" \"\" &&\n+\ttest_completion \"git history reword main -- --update-refs he\" \"\"\n '\n \n test_expect_success 'git history revisions' '\n\n-- \n2.54.0\n\n"},{"id":"549888","messageId":"20260806-history_autocompletion-v2-4-7e60f52a1c20@kernel.org","threadId":"66132","inReplyTo":"20260806-history_autocompletion-v2-0-7e60f52a1c20@kernel.org","subject":"[PATCH v2 4/4] completion: complete 'git history split' pathspecs","fromName":"Vincent Mailhol","fromEmail":"mailhol@kernel.org","sentAt":"2026-08-06T20:27:39Z","receivedAt":"2026-08-06T20:27:54Z","isPatch":true,"body":"Arguments following the required revision of \"git history split\" are\npathspecs. Complete them from tracked paths, including after an explicit\n\"--\".\n\nSigned-off-by: Vincent Mailhol <mailhol@kernel.org>\n---\nChanges in v2:\n\n  - New patch.\n---\n contrib/completion/git-completion.bash |  6 ++++++\n t/t9902-completion.sh                  | 13 +++++++++++++\n 2 files changed, 19 insertions(+)\n\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex 6f1ba96763..d313780d8b 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -2197,6 +2197,12 @@ _git_history ()\n \t\t__git_complete_refs\n \t\treturn\n \tfi\n+\n+\tcase \"$subcommand\" in\n+\tsplit)\n+\t\t__git_complete_index_file \"--cached\"\n+\t\t;;\n+\tesac\n }\n \n _git_init ()\ndiff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\nindex ea86ecc08f..391cc849a8 100755\n--- a/t/t9902-completion.sh\n+++ b/t/t9902-completion.sh\n@@ -3143,6 +3143,19 @@ test_expect_success 'git history revisions' '\n \ttest_completion \"git history reword main m\" \"\"\n '\n \n+test_expect_success 'git history split pathspecs' '\n+\ttest_completion \"git history split main -- --update-refs=h\" \"\" &&\n+\ttest_completion \"git history split main -- --update-refs h\" \"\" &&\n+\ttest_completion \"git history split --dry-run main file\" <<-\\EOF &&\n+\tfile1Z\n+\tfile2Z\n+\tEOF\n+\ttest_completion \"git history split main -- file\" <<-\\EOF\n+\tfile1Z\n+\tfile2Z\n+\tEOF\n+'\n+\n test_expect_success 'git reflog show' '\n \ttest_when_finished \"git checkout - && git branch -d shown\" &&\n \tgit checkout -b shown &&\n\n-- \n2.54.0\n\n"},{"id":"549934","messageId":"anV7cHblfmGvbl-e@pks.im","threadId":"66132","inReplyTo":"20260806-history_autocompletion-v2-1-7e60f52a1c20@kernel.org","subject":"Re: [PATCH v2 1/4] completion: add 'git history' subcommands","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-07T06:30:08Z","receivedAt":"2026-08-07T06:30:13Z","isPatch":true,"body":"On Thu, Aug 06, 2026 at 10:27:36PM +0200, Vincent Mailhol wrote:\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index e875787710..7372e2919b 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -2137,6 +2137,54 @@ _git_help ()\n>  \tfi\n>  }\n>  \n> +__git_history_has_revision ()\n> +{\n> +\tlocal i\n> +\n> +\tfor ((i = __git_cmd_idx + 2; i < cword; i++)); do\n> +\t\tcase \"${words[i]}\" in\n> +\t\t--empty|--update-refs)\n> +\t\t\t((i++))\n> +\t\t\t;;\n\nThis will unfortunately be quite a pain to maintain going forward, as we\nnow have to be aware of updating this site every single time we add a\nnew option that accepts a parameter.\n\nI don't really have a good idea for how to fix that reliably though, I\nhave to admit. Maybe we should just mostly ignore this edge case and\nalways complete references, unless we have seen a `--`? That can be\nchecked rather easily via `__git_hash_doubledash`.\n\nThat'd still be a huge win compared to the status quo, and if we really\ncare about making this work properly we can still iterate.\n\nPatrick\n"},{"id":"549936","messageId":"e894cf4e-7df2-489a-a596-96f1d4d95dc0@kernel.org","threadId":"66132","inReplyTo":"anV7cHblfmGvbl-e@pks.im","subject":"Re: [PATCH v2 1/4] completion: add 'git history' subcommands","fromName":"Vincent Mailhol","fromEmail":"mailhol@kernel.org","sentAt":"2026-08-07T06:44:41Z","receivedAt":"2026-08-07T06:44:45Z","isPatch":true,"body":"On 07/08/2026 at 08:30, Patrick Steinhardt wrote:\n> On Thu, Aug 06, 2026 at 10:27:36PM +0200, Vincent Mailhol wrote:\n>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n>> index e875787710..7372e2919b 100644\n>> --- a/contrib/completion/git-completion.bash\n>> +++ b/contrib/completion/git-completion.bash\n>> @@ -2137,6 +2137,54 @@ _git_help ()\n>>  \tfi\n>>  }\n>>  \n>> +__git_history_has_revision ()\n>> +{\n>> +\tlocal i\n>> +\n>> +\tfor ((i = __git_cmd_idx + 2; i < cword; i++)); do\n>> +\t\tcase \"${words[i]}\" in\n>> +\t\t--empty|--update-refs)\n>> +\t\t\t((i++))\n>> +\t\t\t;;\n> \n> This will unfortunately be quite a pain to maintain going forward, as we\n> now have to be aware of updating this site every single time we add a\n> new option that accepts a parameter.\n\nDo you foreseen such new parameters?\n\n> I don't really have a good idea for how to fix that reliably though, I\n> have to admit. Maybe we should just mostly ignore this edge case and\n> always complete references, unless we have seen a `--`? That can be\n> checked rather easily via `__git_hash_doubledash`.\n\nMy toughs are that if such a special case ever surface, we can just\ndispatch it earlier before we check for the\n__git_history_has_revision, like this:\n\n---8<---\ndiff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\nindex d313780d8b..786fcb5e16 100644\n--- a/contrib/completion/git-completion.bash\n+++ b/contrib/completion/git-completion.bash\n@@ -2193,6 +2193,15 @@ _git_history ()\n \t\tesac\n \tfi\n \n+\t# Subcommands which takes something else than a revision\n+\tcase \"$subcommand\" in\n+\tfoo)\n+\t\t# 'git history foo' take a file first\n+\t\t__git_complete_index_file \"--cached\"\n+\t\treturn\n+\t\t;;\n+\tesac\n+\n \tif ! __git_history_has_revision; then\n \t\t__git_complete_refs\n \t\treturn\n---8<---\n\nThis seems reasonable to me. Once we know what this mysterious new\ncommand would be, maybe we can find a smarter and more tailored\nsolution, but at the moment, I would not call this a blocker.\n\n> That'd still be a huge win compared to the status quo, and if we really\n> care about making this work properly we can still iterate.\n\nThanks!\n\n\nYours sincerely,\nVincent Mailhol\n"},{"id":"549945","messageId":"anWEcfhdzvNQfskU@pks.im","threadId":"66132","inReplyTo":"e894cf4e-7df2-489a-a596-96f1d4d95dc0@kernel.org","subject":"Re: [PATCH v2 1/4] completion: add 'git history' subcommands","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-07T07:08:33Z","receivedAt":"2026-08-07T07:08:39Z","isPatch":true,"body":"On Fri, Aug 07, 2026 at 08:44:41AM +0200, Vincent Mailhol wrote:\n> On 07/08/2026 at 08:30, Patrick Steinhardt wrote:\n> > On Thu, Aug 06, 2026 at 10:27:36PM +0200, Vincent Mailhol wrote:\n> >> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> >> index e875787710..7372e2919b 100644\n> >> --- a/contrib/completion/git-completion.bash\n> >> +++ b/contrib/completion/git-completion.bash\n> >> @@ -2137,6 +2137,54 @@ _git_help ()\n> >>  \tfi\n> >>  }\n> >>  \n> >> +__git_history_has_revision ()\n> >> +{\n> >> +\tlocal i\n> >> +\n> >> +\tfor ((i = __git_cmd_idx + 2; i < cword; i++)); do\n> >> +\t\tcase \"${words[i]}\" in\n> >> +\t\t--empty|--update-refs)\n> >> +\t\t\t((i++))\n> >> +\t\t\t;;\n> > \n> > This will unfortunately be quite a pain to maintain going forward, as we\n> > now have to be aware of updating this site every single time we add a\n> > new option that accepts a parameter.\n> \n> Do you foreseen such new parameters?\n\nYes, I'm very sure we'll gain more parameters for those commands. Commit\nsigning, sign-offs, handling of notes are all things that are currently\nbeing discussed, and they likely will require new options.\n\n> > I don't really have a good idea for how to fix that reliably though, I\n> > have to admit. Maybe we should just mostly ignore this edge case and\n> > always complete references, unless we have seen a `--`? That can be\n> > checked rather easily via `__git_hash_doubledash`.\n> \n> My toughs are that if such a special case ever surface, we can just\n> dispatch it earlier before we check for the\n> __git_history_has_revision, like this:\n> \n> ---8<---\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index d313780d8b..786fcb5e16 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -2193,6 +2193,15 @@ _git_history ()\n>  \t\tesac\n>  \tfi\n>  \n> +\t# Subcommands which takes something else than a revision\n> +\tcase \"$subcommand\" in\n> +\tfoo)\n> +\t\t# 'git history foo' take a file first\n> +\t\t__git_complete_index_file \"--cached\"\n> +\t\treturn\n> +\t\t;;\n> +\tesac\n> +\n>  \tif ! __git_history_has_revision; then\n>  \t\t__git_complete_refs\n>  \t\treturn\n> ---8<---\n> \n> This seems reasonable to me. Once we know what this mysterious new\n> command would be, maybe we can find a smarter and more tailored\n> solution, but at the moment, I would not call this a blocker.\n\nI'm not really concerned about new subcommands for now, true. But\nhardcoding the parameters as we do above feels error prone to me and\nwill very likely diverge as the command evolves.\n\nPatrick\n"},{"id":"549959","messageId":"0ea2cce4-2174-4866-9619-d7f74ae5c91f@kernel.org","threadId":"66132","inReplyTo":"anWEcfhdzvNQfskU@pks.im","subject":"Re: [PATCH v2 1/4] completion: add 'git history' subcommands","fromName":"Vincent Mailhol","fromEmail":"mailhol@kernel.org","sentAt":"2026-08-07T08:09:26Z","receivedAt":"2026-08-07T08:09:30Z","isPatch":true,"body":"On 07/08/2026 at 09:08, Patrick Steinhardt wrote:\n> On Fri, Aug 07, 2026 at 08:44:41AM +0200, Vincent Mailhol wrote:\n>> On 07/08/2026 at 08:30, Patrick Steinhardt wrote:\n>>> On Thu, Aug 06, 2026 at 10:27:36PM +0200, Vincent Mailhol wrote:\n>>>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n>>>> index e875787710..7372e2919b 100644\n>>>> --- a/contrib/completion/git-completion.bash\n>>>> +++ b/contrib/completion/git-completion.bash\n>>>> @@ -2137,6 +2137,54 @@ _git_help ()\n>>>>  \tfi\n>>>>  }\n>>>>  \n>>>> +__git_history_has_revision ()\n>>>> +{\n>>>> +\tlocal i\n>>>> +\n>>>> +\tfor ((i = __git_cmd_idx + 2; i < cword; i++)); do\n>>>> +\t\tcase \"${words[i]}\" in\n>>>> +\t\t--empty|--update-refs)\n>>>> +\t\t\t((i++))\n>>>> +\t\t\t;;\n>>>\n>>> This will unfortunately be quite a pain to maintain going forward, as we\n>>> now have to be aware of updating this site every single time we add a\n>>> new option that accepts a parameter.\n>>\n>> Do you foreseen such new parameters?\n> \n> Yes, I'm very sure we'll gain more parameters for those commands. Commit\n> signing, sign-offs, handling of notes are all things that are currently\n> being discussed, and they likely will require new options.\n\nGot it! I kind of mixed subcommands and parameters in my head. My\nprevious answer was totally off topic, sorry.\n\nFor the new parameters, indeed. The issue is that these options accept\ntwo syntax:\n\n  --empty=<value>\n\nor\n\n  --empty <value>\n\nThe first one falls under the '-*)' switch case anyway, so if you do a\n\n  git history fix --new-option=foo <TAB>\n\nthe __git_history_has_revision will handle it properly. If you do:\n\n  git history fix --new-option=<TAB>\n\nyou just get no completion until the code is modified to teach what are\nthe correct value for --new-option. This is acceptable in term of\nmaintainability.\n\nIf you do:\n\n  git history fix --new-option <TAB>\n\nthen __git_history_has_revision will assume that --new-option is a\ntoggle parameter which takes no value and will incorrectly complete it\nwith a reference.\n\nFinally, if you do a:\n\n  git history fix --new-option value <TAB>\n\nthen the value is interpreted as a reference and the <TAB> gives no\ncompletion.\n\nFor a\n\n  git history fix --gpg-sign\n\nthis is mostly OK. Assuming the new --gpg-sign works identically as the\ngit rebase option, the --gpg-sign value is optional and default the the\ncommitter identity. So in most of the cases, the user will not give a\nvalue and will correctly get the reference completion when doing:\n\n  git history fix --gpg-sign <TAB>\n\nSo the only case where we are screwed is if the option takes an argument\n*and* the user specify it as --new-option (without the final '='). In\nthat case, the damage is still not huge. I expect most of the users to\npass option with the final '='.\n\n>>> I don't really have a good idea for how to fix that reliably though, I\n>>> have to admit. Maybe we should just mostly ignore this edge case and\n>>> always complete references, unless we have seen a `--`? That can be\n>>> checked rather easily via `__git_hash_doubledash`.\n>>\n>> My toughs are that if such a special case ever surface, we can just\n>> dispatch it earlier before we check for the\n>> __git_history_has_revision, like this:\n>>\n>> ---8<---\n>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n>> index d313780d8b..786fcb5e16 100644\n>> --- a/contrib/completion/git-completion.bash\n>> +++ b/contrib/completion/git-completion.bash\n>> @@ -2193,6 +2193,15 @@ _git_history ()\n>>  \t\tesac\n>>  \tfi\n>>  \n>> +\t# Subcommands which takes something else than a revision\n>> +\tcase \"$subcommand\" in\n>> +\tfoo)\n>> +\t\t# 'git history foo' take a file first\n>> +\t\t__git_complete_index_file \"--cached\"\n>> +\t\treturn\n>> +\t\t;;\n>> +\tesac\n>> +\n>>  \tif ! __git_history_has_revision; then\n>>  \t\t__git_complete_refs\n>>  \t\treturn\n>> ---8<---\n>>\n>> This seems reasonable to me. Once we know what this mysterious new\n>> command would be, maybe we can find a smarter and more tailored\n>> solution, but at the moment, I would not call this a blocker.\n> \n> I'm not really concerned about new subcommands for now, true. But\n> hardcoding the parameters as we do above feels error prone to me and\n> will very likely diverge as the command evolves.\n\nI think that there are two options:\n\n  1. What I did, which work great today and will start to diverge the\n     day we add more arguments which takes a value as you highlighted.\n\n  2. Ignore the '--argument <value>' syntax and only complete the\n     '--argument=<value>'.\n\nPoint 2. will consistently give incorrect results when doing:\n\n  git history fix --new-option value <TAB>\n\nbut is easier to maintain. And the '--argument <value>' syntax isn't\ncovered in the manpages anyway, so this option is just a \"we implement\nthe manpages and that's it!\" approach.\n\nMy preference goes slightly to 1., but I am OK to send a v3 with\noption 2.\n\n\nYours sincerely,\nVincent Mailhol\n\n"},{"id":"550170","messageId":"CALnO6CAudrCCr-bZOt5TCo6ZbmxuwE48-Zj-pkcj8Rq2T1-1wg@mail.gmail.com","threadId":"66132","inReplyTo":"0ea2cce4-2174-4866-9619-d7f74ae5c91f@kernel.org","subject":"Re: [PATCH v2 1/4] completion: add 'git history' subcommands","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2026-08-10T12:43:17Z","receivedAt":"2026-08-10T12:43:29Z","isPatch":true,"body":"On Fri, Aug 7, 2026 at 4:13 AM Vincent Mailhol <mailhol@kernel.org> wrote:\n>\n> On 07/08/2026 at 09:08, Patrick Steinhardt wrote:\n> > On Fri, Aug 07, 2026 at 08:44:41AM +0200, Vincent Mailhol wrote:\n> >> On 07/08/2026 at 08:30, Patrick Steinhardt wrote:\n> >>> On Thu, Aug 06, 2026 at 10:27:36PM +0200, Vincent Mailhol wrote:\n> >>>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> >>>> index e875787710..7372e2919b 100644\n> >>>> --- a/contrib/completion/git-completion.bash\n> >>>> +++ b/contrib/completion/git-completion.bash\n> >>>> @@ -2137,6 +2137,54 @@ _git_help ()\n> >>>>    fi\n> >>>>  }\n> >>>>\n> >>>> +__git_history_has_revision ()\n> >>>> +{\n> >>>> +  local i\n> >>>> +\n> >>>> +  for ((i = __git_cmd_idx + 2; i < cword; i++)); do\n> >>>> +          case \"${words[i]}\" in\n> >>>> +          --empty|--update-refs)\n> >>>> +                  ((i++))\n> >>>> +                  ;;\n> >>>\n> >>> This will unfortunately be quite a pain to maintain going forward, as we\n> >>> now have to be aware of updating this site every single time we add a\n> >>> new option that accepts a parameter.\n[snip]\n> > Yes, I'm very sure we'll gain more parameters for those commands. Commit\n> > signing, sign-offs, handling of notes are all things that are currently\n> > being discussed, and they likely will require new options.\n[snip]\n>\n> For the new parameters, indeed. The issue is that these options accept\n> two syntax:\n>\n>   --empty=<value>\n>\n> or\n>\n>   --empty <value>\n>\n> The first one falls under the '-*)' switch case anyway, so if you do a\n>\n>   git history fix --new-option=foo <TAB>\n>\n> the __git_history_has_revision will handle it properly. If you do:\n>\n>   git history fix --new-option=<TAB>\n>\n> you just get no completion until the code is modified to teach what are\n> the correct value for --new-option. This is acceptable in term of\n> maintainability.\n>\n> If you do:\n>\n>   git history fix --new-option <TAB>\n>\n> then __git_history_has_revision will assume that --new-option is a\n> toggle parameter which takes no value and will incorrectly complete it\n> with a reference.\n>\n> Finally, if you do a:\n>\n>   git history fix --new-option value <TAB>\n>\n> then the value is interpreted as a reference and the <TAB> gives no\n> completion.\n>\n> For a\n>\n>   git history fix --gpg-sign\n>\n> this is mostly OK. Assuming the new --gpg-sign works identically as the\n> git rebase option, the --gpg-sign value is optional and default the the\n> committer identity. So in most of the cases, the user will not give a\n> value and will correctly get the reference completion when doing:\n>\n>   git history fix --gpg-sign <TAB>\n>\n> So the only case where we are screwed is if the option takes an argument\n> *and* the user specify it as --new-option (without the final '='). In\n> that case, the damage is still not huge. I expect most of the users to\n> pass option with the final '='.\n\n[later]\n\n> > I'm not really concerned about new subcommands for now, true. But\n> > hardcoding the parameters as we do above feels error prone to me and\n> > will very likely diverge as the command evolves.\n>\n> I think that there are two options:\n>\n>   1. What I did, which work great today and will start to diverge the\n>      day we add more arguments which takes a value as you highlighted.\n>\n>   2. Ignore the '--argument <value>' syntax and only complete the\n>      '--argument=<value>'.\n>\n> Point 2. will consistently give incorrect results when doing:\n>\n>   git history fix --new-option value <TAB>\n>\n> but is easier to maintain. And the '--argument <value>' syntax isn't\n> covered in the manpages anyway, so this option is just a \"we implement\n> the manpages and that's it!\" approach.\n>\n> My preference goes slightly to 1., but I am OK to send a v3 with\n> option 2.\n\n- The manuals (gitcli, especially) recommend the stuck form (-oArg,\n--long-opt=Arg)\n- Completion code that I'm aware of completes the string \"--long-opt=\"\n\nSo I suspect most folks using completion will end up with the stuck\nform. If we want to support the unstuck form, I'm ok with that\n(Vincent's (1)). It seems simpler for now to go with (2), which aligns\nwith the rest of the codebase, and wait to see if anyone complains\nthough.\n\nSwitching topics:\n\n> >>> I don't really have a good idea for how to fix that reliably though, I\n> >>> have to admit. Maybe we should just mostly ignore this edge case and\n> >>> always complete references, unless we have seen a `--`? That can be\n> >>> checked rather easily via `__git_hash_doubledash`.\n> >>\n> >> My toughs are that if such a special case ever surface, we can just\n> >> dispatch it earlier before we check for the\n> >> __git_history_has_revision, like this:\n[snip]\n> >>\n> >> This seems reasonable to me. Once we know what this mysterious new\n> >> command would be, maybe we can find a smarter and more tailored\n> >> solution, but at the moment, I would not call this a blocker.\n\nThis is similar to what we do stash and a few other\nsubcommand-commands, where we need to dispatch a bit differently. I\nthink trying to assume all git-history commands will have the same\nshape is both pleasant (consistent interface!) and unlikely to hold up\n(something will diverge somewhere).\n\n-- \nD. Ben Knoble\n"},{"id":"550172","messageId":"CALnO6CCCG0xcZtAKQdNsKxNJ2Nyq5HztLaz_7QXjfQsN-q-xgA@mail.gmail.com","threadId":"66132","inReplyTo":"20260806-history_autocompletion-v2-2-7e60f52a1c20@kernel.org","subject":"Re: [PATCH v2 2/4] completion: complete 'git history --empty' values","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2026-08-10T12:48:39Z","receivedAt":"2026-08-10T12:48:51Z","isPatch":true,"body":"On Thu, Aug 6, 2026 at 4:36 PM Vincent Mailhol <mailhol@kernel.org> wrote:\n>\n> The \"--empty\" option accepts \"drop\", \"keep\", or \"abort\" for the \"drop\"\n> and \"fixup\" subcommands. Complete these values.\n>\n> Although the synopsis only documents the:\n>\n>   --empty=<value>\n>\n> form, parse-options also accepts the value as a separate argument:\n>\n>   --empty <value>\n>\n> Support both forms to follow the parser.\n\nComments on 1/4 apply here, too. I don't mind supporting both, but I\nwonder if we should be consistent with gitcli(1) and just go with the\nstuck form.\n\nI can only find one hit for the pattern \"--[[:alnum:]-]+[^=],?\\*\" (use\n\"git grep -E\") in the completion code, and it's \"--no-*)\", so I'm not\nsure if other commands support completing the unstuck form? For\nexample, \"git commit --cleanup <tab>\" doesn't complete the mode\nargument, but \"git commit --cleanup=<tab>\" does.\n\n>\n> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>\n> ---\n> Changes in v2:\n>\n>   - New patch.\n> ---\n>  contrib/completion/git-completion.bash | 13 +++++++++++--\n>  t/t9902-completion.sh                  |  5 ++++-\n>  2 files changed, 15 insertions(+), 3 deletions(-)\n>\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 7372e2919b..fe5223b8ec 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -2171,8 +2171,17 @@ _git_history ()\n>         fi\n>\n>         if ! __git_has_doubledash; then\n> -               case \"$cur\" in\n> -               --*)\n> +               case \"$prev,$cur\" in\n> +               --empty,*|*,--empty=*)\n> +                       case \"$subcommand\" in\n> +                       drop|fixup)\n> +                               __gitcomp \"drop keep abort\" \"\" \\\n> +                                       \"${cur##--empty=}\"\n> +                               return\n> +                               ;;\n> +                       esac\n> +                       ;;\n> +               *,--*)\n>                         __gitcomp_builtin \"history_$subcommand\"\n>                         return\n>                         ;;\n> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh\n> index 5ccb38c751..52a036a1ad 100755\n> --- a/t/t9902-completion.sh\n> +++ b/t/t9902-completion.sh\n> @@ -3126,7 +3126,10 @@ test_expect_success 'git history subcommand options' '\n>         test_completion \"git history fixup --ree\" \"--reedit-message \" &&\n>         test_completion \"git history split --upd\" \"--update-refs=\" &&\n>         test_completion \"git history split main --dry\" \"--dry-run \" &&\n> -       test_completion \"git history reword main -- --d\" \"\"\n> +       test_completion \"git history reword main -- --d\" \"\" &&\n> +       test_completion \"git history fixup --empty=ke\" \"keep \" &&\n> +       test_completion \"git history drop --empty ab\" \"abort \" &&\n> +       test_completion \"git history reword --empty=ke\" \"\"\n>  '\n>\n>  test_expect_success 'git history revisions' '\n>\n> --\n> 2.54.0\n>\n>\n\n\n-- \nD. Ben Knoble\n"},{"id":"550175","messageId":"CALnO6CAssyDe7uOK+G8eZPzu1S6iyn8EiSQGqUHtWgdPcD65xw@mail.gmail.com","threadId":"66132","inReplyTo":"20260806-history_autocompletion-v2-2-7e60f52a1c20@kernel.org","subject":"Re: [PATCH v2 2/4] completion: complete 'git history --empty' values","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2026-08-10T12:50:58Z","receivedAt":"2026-08-10T12:51:09Z","isPatch":true,"body":"One other thing, sorry\n\nOn Thu, Aug 6, 2026 at 4:36 PM Vincent Mailhol <mailhol@kernel.org> wrote:\n>\n> The \"--empty\" option accepts \"drop\", \"keep\", or \"abort\" for the \"drop\"\n> and \"fixup\" subcommands. Complete these values.\n>\n> Although the synopsis only documents the:\n>\n>   --empty=<value>\n>\n> form, parse-options also accepts the value as a separate argument:\n>\n>   --empty <value>\n>\n> Support both forms to follow the parser.\n>\n> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>\n> ---\n> Changes in v2:\n>\n>   - New patch.\n> ---\n>  contrib/completion/git-completion.bash | 13 +++++++++++--\n>  t/t9902-completion.sh                  |  5 ++++-\n>  2 files changed, 15 insertions(+), 3 deletions(-)\n>\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 7372e2919b..fe5223b8ec 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -2171,8 +2171,17 @@ _git_history ()\n>         fi\n>\n>         if ! __git_has_doubledash; then\n> -               case \"$cur\" in\n> -               --*)\n> +               case \"$prev,$cur\" in\n> +               --empty,*|*,--empty=*)\n> +                       case \"$subcommand\" in\n> +                       drop|fixup)\n\nThis feels a bit \"inside out\" to me, especially when reading the other\ncompletions. I think the usual pattern is to check the subcommand\nfirst and dispatch if necessary. Thoughts?\n\n> +                               __gitcomp \"drop keep abort\" \"\" \\\n> +                                       \"${cur##--empty=}\"\n> +                               return\n> +                               ;;\n> +                       esac\n> +                       ;;\n> +               *,--*)\n>                         __gitcomp_builtin \"history_$subcommand\"\n>                         return\n>                         ;;\n[snip]\n\n\n\n-- \nD. Ben Knoble\n"},{"id":"550176","messageId":"CALnO6CDZURfK3HFQF_LYrSz0KWtamUguVWK3-cnVUCeA+oVBHQ@mail.gmail.com","threadId":"66132","inReplyTo":"20260806-history_autocompletion-v2-3-7e60f52a1c20@kernel.org","subject":"Re: [PATCH v2 3/4] completion: complete 'git history --update-refs' values","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2026-08-10T12:52:51Z","receivedAt":"2026-08-10T12:53:03Z","isPatch":true,"body":"On Thu, Aug 6, 2026 at 4:37 PM Vincent Mailhol <mailhol@kernel.org> wrote:\n>\n> The \"--update-refs\" option accepts either \"branches\" or \"head\".\n> Complete these values.\n>\n> Although the synopsis only documents the:\n>\n>   --update-refs=<value>\n>\n> form, parse-options also accepts the value as a separate argument:\n>\n>   --update-refs <value>\n>\n> Support both forms to follow the parser.\n>\n> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>\n> ---\n> Changes in v2:\n>\n>   - New patch.\n> ---\n>  contrib/completion/git-completion.bash | 5 +++++\n>  t/t9902-completion.sh                  | 6 +++++-\n>  2 files changed, 10 insertions(+), 1 deletion(-)\n>\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index fe5223b8ec..6f1ba96763 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -2181,6 +2181,11 @@ _git_history ()\n>                                 ;;\n>                         esac\n>                         ;;\n> +               --update-refs,*|*,--update-refs=*)\n> +                       __gitcomp \"branches head\" \"\" \\\n> +                               \"${cur##--update-refs=}\"\n> +                       return\n> +                       ;;\n\nContrary to my comments on 2/4, this seems like a reasonable place for\n--update-refs, since that applies to all current git-history commands.\nIf that ever changes, well… we'll deal with it then I suppose.\n\n>                 *,--*)\n>                         __gitcomp_builtin \"history_$subcommand\"\n>                         return\n[snip]\n\n\n-- \nD. Ben Knoble\n"},{"id":"550177","messageId":"CALnO6CBThicX2x_acKoSvWMOkr4pa5bVMH=RNMXO+BjEAxKSHg@mail.gmail.com","threadId":"66132","inReplyTo":"20260806-history_autocompletion-v2-4-7e60f52a1c20@kernel.org","subject":"Re: [PATCH v2 4/4] completion: complete 'git history split' pathspecs","fromName":"D. Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2026-08-10T12:58:32Z","receivedAt":"2026-08-10T12:58:43Z","isPatch":true,"body":"On Thu, Aug 6, 2026 at 4:37 PM Vincent Mailhol <mailhol@kernel.org> wrote:\n>\n> Arguments following the required revision of \"git history split\" are\n> pathspecs. Complete them from tracked paths, including after an explicit\n> \"--\".\n>\n> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>\n> ---\n> Changes in v2:\n>\n>   - New patch.\n> ---\n>  contrib/completion/git-completion.bash |  6 ++++++\n>  t/t9902-completion.sh                  | 13 +++++++++++++\n>  2 files changed, 19 insertions(+)\n>\n> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> index 6f1ba96763..d313780d8b 100644\n> --- a/contrib/completion/git-completion.bash\n> +++ b/contrib/completion/git-completion.bash\n> @@ -2197,6 +2197,12 @@ _git_history ()\n>                 __git_complete_refs\n>                 return\n>         fi\n> +\n> +       case \"$subcommand\" in\n> +       split)\n> +               __git_complete_index_file \"--cached\"\n> +               ;;\n> +       esac\n\nIn context, this seems late to me relative to other completion functions:\n\n- complete subcommands\n- special case a few options\n- handle revisions\n- split\n\nvs., say, _git_notes, _git_reflog, etc. where the pattern is\n\n    case \"$subcommand,$cur\" in\n\nto dispatch on combinations. We could use \"split,*)\" to dispatch there.\n\nOTOH! The split completion wants to benefit from the other things done\n(like revision completion), and only then (before or after --)\ndelegate to pathspecs. So, I dunno: I think this location achieves\nthat goal, but it diverges somewhat from the way other completions are\nwritten.\n\n[snip]\n\n-- \nD. Ben Knoble\n"},{"id":"550479","messageId":"CAMZ6RqLAYMSwNK=w=Xh+O==46eQCS=wFgBoUEOtQoBbLrBqd_A@mail.gmail.com","threadId":"66132","inReplyTo":"CALnO6CAssyDe7uOK+G8eZPzu1S6iyn8EiSQGqUHtWgdPcD65xw@mail.gmail.com","subject":"Re: [PATCH v2 2/4] completion: complete 'git history --empty' values","fromName":"Vincent Mailhol","fromEmail":"mailhol@kernel.org","sentAt":"2026-08-13T08:20:00Z","receivedAt":"2026-08-13T08:20:13Z","isPatch":true,"body":"On Mon. 10 Aug. 2026 at 14:50, D. Ben Knoble <ben.knoble@gmail.com> wrote:\n> One other thing, sorry\n>\n> On Thu, Aug 6, 2026 at 4:36 PM Vincent Mailhol <mailhol@kernel.org> wrote:\n> >\n> > The \"--empty\" option accepts \"drop\", \"keep\", or \"abort\" for the \"drop\"\n> > and \"fixup\" subcommands. Complete these values.\n> >\n> > Although the synopsis only documents the:\n> >\n> >   --empty=<value>\n> >\n> > form, parse-options also accepts the value as a separate argument:\n> >\n> >   --empty <value>\n> >\n> > Support both forms to follow the parser.\n> >\n> > Signed-off-by: Vincent Mailhol <mailhol@kernel.org>\n> > ---\n> > Changes in v2:\n> >\n> >   - New patch.\n> > ---\n> >  contrib/completion/git-completion.bash | 13 +++++++++++--\n> >  t/t9902-completion.sh                  |  5 ++++-\n> >  2 files changed, 15 insertions(+), 3 deletions(-)\n> >\n> > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> > index 7372e2919b..fe5223b8ec 100644\n> > --- a/contrib/completion/git-completion.bash\n> > +++ b/contrib/completion/git-completion.bash\n> > @@ -2171,8 +2171,17 @@ _git_history ()\n> >         fi\n> >\n> >         if ! __git_has_doubledash; then\n> > -               case \"$cur\" in\n> > -               --*)\n> > +               case \"$prev,$cur\" in\n> > +               --empty,*|*,--empty=*)\n> > +                       case \"$subcommand\" in\n> > +                       drop|fixup)\n>\n> This feels a bit \"inside out\" to me, especially when reading the other\n> completions. I think the usual pattern is to check the subcommand\n> first and dispatch if necessary. Thoughts?\n\nThe motivation is to have a single:\n\n  case \"$cur\" in\n\nstatement.\n\nAfter dropping support for the separated option-value form, this is\nwhat the code looks like if we check the subcommand first and the\noption second:\n\n        if ! __git_has_doubledash; then\n                case \"$subcommand\" in\n                drop|fixup)\n                        case \"$cur\" in\n                        --empty=*)\n                                __gitcomp \"drop keep abort\" \"\" \\\n                                        \"${cur##--empty=}\"\n                                return\n                                ;;\n                        esac\n                        ;;\n                esac\n\n                case \"$cur\" in\n                --update-refs=*)\n                        __gitcomp \"branches head\" \"\" \\\n                                \"${cur##--update-refs=}\"\n                        return\n                        ;;\n                --*)\n                        __gitcomp_builtin \"history_$subcommand\"\n                        return\n                        ;;\n                esac\n        fi\n\nSee the repeated 'case \"$cur\" in'. Note that it is not possible to\nhave a wild card *) in the first switch case unless the\n--update-refs=*) dispatch gets duplicated. In the end, by using this\napproach, one part of the code will need to get duplicated.\n\nOn the contrary, by dispatching the option first and the subcommand\nsecond like this:\n\n        if ! __git_has_doubledash; then\n                case \"$cur\" in\n                --empty=*)\n                        case \"$subcommand\" in\n                        drop|fixup)\n                                __gitcomp \"drop keep abort\" \"\" \\\n                                        \"${cur##--empty=}\"\n                                ;;\n                        esac\n                        return\n                        ;;\n                --update-refs=*)\n                        __gitcomp \"branches head\" \"\" \\\n                                \"${cur##--update-refs=}\"\n                        return\n                        ;;\n                --*)\n                        __gitcomp_builtin \"history_$subcommand\"\n                        return\n                        ;;\n                esac\n        fi\n\nwe do not see the conflict and do not need to repeat any of the switch cases.\n\n> > +                               __gitcomp \"drop keep abort\" \"\" \\\n> > +                                       \"${cur##--empty=}\"\n> > +                               return\n> > +                               ;;\n> > +                       esac\n> > +                       ;;\n> > +               *,--*)\n> >                         __gitcomp_builtin \"history_$subcommand\"\n> >                         return\n> >                         ;;\n> [snip]\n\n\nYours sincerely,\nVincent Mailhol\n"},{"id":"550497","messageId":"00E5CBDB-7D2A-4117-9A52-FD5C64A9838C@gmail.com","threadId":"66132","inReplyTo":"CAMZ6RqLAYMSwNK=w=Xh+O==46eQCS=wFgBoUEOtQoBbLrBqd_A@mail.gmail.com","subject":"Re: [PATCH v2 2/4] completion: complete 'git history --empty' values","fromName":"Ben Knoble","fromEmail":"ben.knoble@gmail.com","sentAt":"2026-08-13T11:12:48Z","receivedAt":"2026-08-13T11:13:01Z","isPatch":true,"body":"\n> Le 13 août 2026 à 04:20, Vincent Mailhol <mailhol@kernel.org> a écrit :\n> \n> On Mon. 10 Aug. 2026 at 14:50, D. Ben Knoble <ben.knoble@gmail.com> wrote:\n>> One other thing, sorry\n>> \n>>> On Thu, Aug 6, 2026 at 4:36 PM Vincent Mailhol <mailhol@kernel.org> wrote:\n>>> \n>>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n>>> index 7372e2919b..fe5223b8ec 100644\n>>> --- a/contrib/completion/git-completion.bash\n>>> +++ b/contrib/completion/git-completion.bash\n>>> @@ -2171,8 +2171,17 @@ _git_history ()\n>>>        fi\n>>> \n>>>        if ! __git_has_doubledash; then\n>>> -               case \"$cur\" in\n>>> -               --*)\n>>> +               case \"$prev,$cur\" in\n>>> +               --empty,*|*,--empty=*)\n>>> +                       case \"$subcommand\" in\n>>> +                       drop|fixup)\n>> \n>> This feels a bit \"inside out\" to me, especially when reading the other\n>> completions. I think the usual pattern is to check the subcommand\n>> first and dispatch if necessary. Thoughts?\n> \n> The motivation is to have a single:\n> \n>  case \"$cur\" in\n> \n> statement.\n\nI now suspect this is why some use the « case \"$subcommand,$cur\" » variant ? Apologies for not thinking of that previously. "},{"id":"550505","messageId":"CAMZ6Rq+mBKHE=mNd9QQOWfpuDQwcMK7qZ2jn1tTPdFJEkUrGOQ@mail.gmail.com","threadId":"66132","inReplyTo":"00E5CBDB-7D2A-4117-9A52-FD5C64A9838C@gmail.com","subject":"Re: [PATCH v2 2/4] completion: complete 'git history --empty' values","fromName":"Vincent Mailhol","fromEmail":"mailhol@kernel.org","sentAt":"2026-08-13T13:45:03Z","receivedAt":"2026-08-13T13:45:15Z","isPatch":true,"body":"On Thu. 13 Aug. 2026 at 13:12, Ben Knoble <ben.knoble@gmail.com> wrote:\n> > Le 13 août 2026 à 04:20, Vincent Mailhol <mailhol@kernel.org> a écrit :\n> >\n> > On Mon. 10 Aug. 2026 at 14:50, D. Ben Knoble <ben.knoble@gmail.com> wrote:\n> >> One other thing, sorry\n> >>\n> >>> On Thu, Aug 6, 2026 at 4:36 PM Vincent Mailhol <mailhol@kernel.org> wrote:\n> >>>\n> >>> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash\n> >>> index 7372e2919b..fe5223b8ec 100644\n> >>> --- a/contrib/completion/git-completion.bash\n> >>> +++ b/contrib/completion/git-completion.bash\n> >>> @@ -2171,8 +2171,17 @@ _git_history ()\n> >>>        fi\n> >>>\n> >>>        if ! __git_has_doubledash; then\n> >>> -               case \"$cur\" in\n> >>> -               --*)\n> >>> +               case \"$prev,$cur\" in\n> >>> +               --empty,*|*,--empty=*)\n> >>> +                       case \"$subcommand\" in\n> >>> +                       drop|fixup)\n> >>\n> >> This feels a bit \"inside out\" to me, especially when reading the other\n> >> completions. I think the usual pattern is to check the subcommand\n> >> first and dispatch if necessary. Thoughts?\n> >\n> > The motivation is to have a single:\n> >\n> >  case \"$cur\" in\n> >\n> > statement.\n>\n> I now suspect this is why some use the « case \"$subcommand,$cur\" » variant ?\n\nIMHO,\n\n  case \"$subcommand,$cur\"\n\nis not very elegant. Sometimes, it is a good trade-off, but here, it\ndoes not seem to be the best solution. Of course, maybe some future\nchanges in git history would make this a preferable option, but I do\nnot have a crystal ball to predict the future.\n\n> Apologies for not thinking of that previously.\n\nNo problem :)\n\n\nYours sincerely,\nVincent Mailhol\n"}]}