# [PATCH 0/7] completion bash: add more options and commands 41 messages from 2017-01-22 to 2017-02-02. Participants: bitte.keine.werbung.einwerfen@googlemail.com, Johannes Sixt, Cornelius Weig, Junio C Hamano, Stefan Beller, Linus Torvalds, Eric Wong, Philip Oakley, cornelius.weig@tngtech.com, SZEDER Gábor. Thread: https://gitlist.dev/t/44947 ## bitte.keine.werbung.einwerfen@googlemail.com, 2017-01-22 22:57 Subject: [PATCH 0/7] completion bash: add more options and commands Message-ID: <20170122225724.19360-1-cornelius.weig@tngtech.com> URL: https://gitlist.dev/e/20170122225724.19360-1-cornelius.weig%40tngtech.com ``` From: Cornelius Weig The cli interface for git has the greatest versatility of all git tools. Its productivity depends on how easy it is to type commands and to remember options. The command line completion helps two-fold by easing input and displaying possible options. Bash completion lacks support for several options and some useful commands. Although the selection of recognized options is always opinionated (e.g. the documentation for git-completion.bash says that 'common' long options are recognized), I think that extending the cli completion will always increase the productivity. To that end, the following commits add completion support for several existing options and subcommands. For every command all options that are mentioned in the introduction of its man page should now be available for completion. Cornelius Weig (7): completion: teach options to submodule subcommands completion: add subcommand completion for rerere completion: improve bash completion for git-add completion: teach ls-remote to complete options completion: teach replace to complete options completion: teach remote subcommands option completion completion: recognize more long-options contrib/completion/git-completion.bash | 132 +++++++++++++++++++++++++++------ 1 file changed, 110 insertions(+), 22 deletions(-) -- 2.10.2 ``` ## bitte.keine.werbung.einwerfen@googlemail.com, 2017-01-22 22:57 Subject: [PATCH 1/7] completion: teach options to submodule subcommands Message-ID: <20170122225724.19360-2-cornelius.weig@tngtech.com> URL: https://gitlist.dev/e/20170122225724.19360-2-cornelius.weig%40tngtech.com In-Reply-To: <20170122225724.19360-1-cornelius.weig@tngtech.com> ``` From: Cornelius Weig The subcommands of submodule have different long-options which command line users need to type in. Therefore, teach bash completion to support most subcommand options for submodule. --- contrib/completion/git-completion.bash | 32 ++++++++++++++++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 6721ff8..c54a557 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2556,10 +2556,11 @@ _git_submodule () __git_has_doubledash && return local subcommands="add status init deinit update summary foreach sync" - if [ -z "$(__git_find_on_cmdline "$subcommands")" ]; then + local subcommand="$(__git_find_on_cmdline "$subcommands")" + if [ -z "$subcommand" ]; then case "$cur" in --*) - __gitcomp "--quiet --cached" + __gitcomp "--quiet" ;; *) __gitcomp "$subcommands" @@ -2567,6 +2568,33 @@ _git_submodule () esac return fi + + case "$subcommand,$cur" in + add,--*) + __gitcomp "--branch --force --name --reference --depth" + ;; + status,--*) + __gitcomp "--cached --recursive" + ;; + deinit,--*) + __gitcomp "--force --all" + ;; + update,--*) + __gitcomp " + --init --remote --no-fetch + --recommend-shallow --no-recommend-shallow + --force --rebase --merge --reference --depth --recursive --jobs + " + ;; + summary,--*) + __gitcomp "--cached --files --summary-limit" + ;; + foreach,--*|sync,--*) + __gitcomp "--recursive" + ;; + *) + ;; + esac } _git_svn () -- 2.10.2 ``` ## bitte.keine.werbung.einwerfen@googlemail.com, 2017-01-22 22:57 Subject: [PATCH 2/7] completion: add subcommand completion for rerere Message-ID: <20170122225724.19360-3-cornelius.weig@tngtech.com> URL: https://gitlist.dev/e/20170122225724.19360-3-cornelius.weig%40tngtech.com In-Reply-To: <20170122225724.19360-1-cornelius.weig@tngtech.com> ``` From: Cornelius Weig Managing recorded resolutions requires command-line usage of git-rerere. Added subcommand completion for rerere and path completion for its subcommand forget. --- contrib/completion/git-completion.bash | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index c54a557..8329f09 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2401,6 +2401,17 @@ _git_replace () __gitcomp_nl "$(__git_refs)" } +_git_rerere () +{ + local subcommands="clear forget diff remaining status gc" + local subcommand="$(__git_find_on_cmdline "$subcommands")" + if test -z "$subcommand" + then + __gitcomp "$subcommands" + return + fi +} + _git_reset () { __git_has_doubledash && return -- 2.10.2 ``` ## bitte.keine.werbung.einwerfen@googlemail.com, 2017-01-22 22:57 Subject: [PATCH 4/7] completion: teach ls-remote to complete options Message-ID: <20170122225724.19360-5-cornelius.weig@tngtech.com> URL: https://gitlist.dev/e/20170122225724.19360-5-cornelius.weig%40tngtech.com In-Reply-To: <20170122225724.19360-1-cornelius.weig@tngtech.com> ``` From: Cornelius Weig ls-remote needs to complete remote names and its own options. In addition to the existing remote name completions, do also complete the options --heads, --tags, --refs, and --get-url. --- contrib/completion/git-completion.bash | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 652c7e2..36fe439 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -1449,6 +1449,12 @@ _git_ls_files () _git_ls_remote () { + case "$cur" in + --*) + __gitcomp "--heads --tags --refs --get-url" + return + ;; + esac __gitcomp_nl "$(__git_remotes)" } -- 2.10.2 ``` ## bitte.keine.werbung.einwerfen@googlemail.com, 2017-01-22 22:57 Subject: [PATCH 7/7] completion: recognize more long-options Message-ID: <20170122225724.19360-8-cornelius.weig@tngtech.com> URL: https://gitlist.dev/e/20170122225724.19360-8-cornelius.weig%40tngtech.com In-Reply-To: <20170122225724.19360-1-cornelius.weig@tngtech.com> ``` From: Cornelius Weig Recognize several new long-options for bash completion in the following commands: - apply: --recount --directory= --unsafe-paths - archive: --prefix= --remote= --exec= --output - branch: --column --no-column --sort= --points-at - clone: --no-single-branch --shallow-submodules - commit: --patch --short --date --allow-empty - describe: --first-parent - fetch, pull: --unshallow --update-shallow - fsck: --name-objects - grep: --break --heading --show-function --function-context --untracked --no-index - mergetool: --prompt --no-prompt - reset: --merge --mixed --hard --soft --patch --keep - revert: --strategy= --strategy-option= - rm: --force - shortlog: --email - tag: --merged --no-merged --create-reflog --- contrib/completion/git-completion.bash | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 0e09519..87d3d2c 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -936,6 +936,7 @@ _git_apply () --apply --no-add --exclude= --ignore-whitespace --ignore-space-change --whitespace= --inaccurate-eof --verbose + --recount --directory= --unsafe-paths " return esac @@ -974,7 +975,7 @@ _git_archive () --*) __gitcomp " --format= --list --verbose - --prefix= --remote= --exec= + --prefix= --remote= --exec= --output " return ;; @@ -1029,6 +1030,7 @@ _git_branch () --track --no-track --contains --merged --no-merged --set-upstream-to= --edit-description --list --unset-upstream --delete --move --remotes + --column --no-column --sort= --points-at " ;; *) @@ -1142,6 +1144,8 @@ _git_clone () --single-branch --branch --recurse-submodules + --no-single-branch + --shallow-submodules " return ;; @@ -1183,6 +1187,7 @@ _git_commit () --reset-author --file= --message= --template= --cleanup= --untracked-files --untracked-files= --verbose --quiet --fixup= --squash= + --patch --short --date --allow-empty " return esac @@ -1201,7 +1206,7 @@ _git_describe () --*) __gitcomp " --all --tags --contains --abbrev= --candidates= - --exact-match --debug --long --match --always + --exact-match --debug --long --match --always --first-parent " return esac @@ -1284,6 +1289,7 @@ __git_fetch_recurse_submodules="yes on-demand no" __git_fetch_options=" --quiet --verbose --append --upload-pack --force --keep --depth= --tags --no-tags --all --prune --dry-run --recurse-submodules= + --unshallow --update-shallow " _git_fetch () @@ -1333,7 +1339,7 @@ _git_fsck () --*) __gitcomp " --tags --root --unreachable --cache --no-reflogs --full - --strict --verbose --lost-found + --strict --verbose --lost-found --name-objects " return ;; @@ -1377,6 +1383,8 @@ _git_grep () --max-depth --count --and --or --not --all-match + --break --heading --show-function --function-context + --untracked --no-index " return ;; @@ -1576,7 +1584,7 @@ _git_mergetool () return ;; --*) - __gitcomp "--tool=" + __gitcomp "--tool= --prompt --no-prompt" return ;; esac @@ -2456,7 +2464,7 @@ _git_reset () case "$cur" in --*) - __gitcomp "--merge --mixed --hard --soft --patch" + __gitcomp "--merge --mixed --hard --soft --patch --keep" return ;; esac @@ -2472,7 +2480,10 @@ _git_revert () fi case "$cur" in --*) - __gitcomp "--edit --mainline --no-edit --no-commit --signoff" + __gitcomp " + --edit --mainline --no-edit --no-commit --signoff + --strategy= --strategy-option= + " return ;; esac @@ -2483,7 +2494,7 @@ _git_rm () { case "$cur" in --*) - __gitcomp "--cached --dry-run --ignore-unmatch --quiet" + __gitcomp "--cached --dry-run --ignore-unmatch --quiet --force" return ;; esac @@ -2500,7 +2511,7 @@ _git_shortlog () __gitcomp " $__git_log_common_options $__git_log_shortlog_options - --numbered --summary + --numbered --summary --email " return ;; @@ -2778,8 +2789,8 @@ _git_tag () --*) __gitcomp " --list --delete --verify --annotate --message --file - --sign --cleanup --local-user --force --column --sort - --contains --points-at + --sign --cleanup --local-user --force --column --sort= + --contains --points-at --merged --no-merged --create-reflog " ;; esac -- 2.10.2 ``` ## bitte.keine.werbung.einwerfen@googlemail.com, 2017-01-22 22:57 Subject: [PATCH 3/7] completion: improve bash completion for git-add Message-ID: <20170122225724.19360-4-cornelius.weig@tngtech.com> URL: https://gitlist.dev/e/20170122225724.19360-4-cornelius.weig%40tngtech.com In-Reply-To: <20170122225724.19360-1-cornelius.weig@tngtech.com> ``` From: Cornelius Weig Add some long-options for git-add and improve path completion when the --update flag is given. --- contrib/completion/git-completion.bash | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 8329f09..652c7e2 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -947,13 +947,17 @@ _git_add () --*) __gitcomp " --interactive --refresh --patch --update --dry-run - --ignore-errors --intent-to-add + --ignore-errors --intent-to-add --force --edit --chmod= " return esac - # XXX should we check for --update and --all options ? - __git_complete_index_file "--others --modified --directory --no-empty-directory" + local complete_opt="--others --modified --directory --no-empty-directory" + if test -n "$(__git_find_on_cmdline "-u --update")" + then + complete_opt="--modified" + fi + __git_complete_index_file "$complete_opt" } _git_archive () -- 2.10.2 ``` ## bitte.keine.werbung.einwerfen@googlemail.com, 2017-01-22 22:57 Subject: [PATCH 5/7] completion: teach replace to complete options Message-ID: <20170122225724.19360-6-cornelius.weig@tngtech.com> URL: https://gitlist.dev/e/20170122225724.19360-6-cornelius.weig%40tngtech.com In-Reply-To: <20170122225724.19360-1-cornelius.weig@tngtech.com> ``` From: Cornelius Weig Git-replace needs to complete references and its own options. In addition to the existing references completions, do also complete the options --edit --graft --format= --list --delete. --- contrib/completion/git-completion.bash | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 36fe439..e76cbd7 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2408,6 +2408,12 @@ _git_remote () _git_replace () { + case "$cur" in + --*) + __gitcomp "--edit --graft --format= --list --delete" + return + ;; + esac __gitcomp_nl "$(__git_refs)" } -- 2.10.2 ``` ## bitte.keine.werbung.einwerfen@googlemail.com, 2017-01-22 22:57 Subject: [PATCH 6/7] completion: teach remote subcommands option completion Message-ID: <20170122225724.19360-7-cornelius.weig@tngtech.com> URL: https://gitlist.dev/e/20170122225724.19360-7-cornelius.weig%40tngtech.com In-Reply-To: <20170122225724.19360-1-cornelius.weig@tngtech.com> ``` From: Cornelius Weig Git-remote needs to complete remote names, its subcommands, and options thereof. In addition to the existing subcommand and remote name completion, do also complete the options - add: --track --master --fetch --tags --no-tags --mirror= - set-url: --push --add --delete - get-url: --push --all - prune: --dry-run --- contrib/completion/git-completion.bash | 36 +++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index e76cbd7..0e09519 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2384,24 +2384,46 @@ _git_config () _git_remote () { - local subcommands="add rename remove set-head set-branches set-url show prune update" + local subcommands=" + add rename remove set-head set-branches + get-url set-url show prune update + " local subcommand="$(__git_find_on_cmdline "$subcommands")" if [ -z "$subcommand" ]; then - __gitcomp "$subcommands" + case "$cur" in + --*) + __gitcomp "--verbose" + ;; + *) + __gitcomp "$subcommands" + ;; + esac return fi - case "$subcommand" in - rename|remove|set-url|show|prune) - __gitcomp_nl "$(__git_remotes)" + case "$subcommand,$cur" in + add,--*) + __gitcomp "--track --master --fetch --tags --no-tags --mirror=" ;; - set-head|set-branches) + add,*) + ;; + set-head,*|set-branches,*) __git_complete_remote_or_refspec ;; - update) + update,*) __gitcomp "$(__git_get_config_variables "remotes")" ;; + set-url,--*) + __gitcomp "--push --add --delete" + ;; + get-url,--*) + __gitcomp "--push --all" + ;; + prune,--*) + __gitcomp "--dry-run" + ;; *) + __gitcomp_nl "$(__git_remotes)" ;; esac } -- 2.10.2 ``` ## Johannes Sixt, 2017-01-24 07:15 Subject: Re: [PATCH 7/7] completion: recognize more long-options Message-ID: <74ecd09c-55da-3858-5187-52c286a6bf62@kdbg.org> URL: https://gitlist.dev/e/74ecd09c-55da-3858-5187-52c286a6bf62%40kdbg.org In-Reply-To: <20170122225724.19360-8-cornelius.weig@tngtech.com> ``` If at all possible, please use your real email address as the From address. It is pointless to hide behind a fake address because as Git contributor you will have to reveal your identity anyway. Please study item (5) "Sign your work" in Documentation/SubmittingPatches and sign off your work. I'm by no means a bash completion expert, but I have a comment on this patch. Am 22.01.2017 um 23:57 schrieb bitte.keine.werbung.einwerfen@googlemail.com: > Recognize several new long-options for bash completion in the following > commands: AFAIR, it was a deliberate decision that potentially destructive command line options are not included in command completions. In the list given, I find these: > - apply: --unsafe-paths > - reset: --merge --mixed --hard --soft --patch --keep > - rm: --force Additionally, these options are only for internal purposes, but I may be wrong: > - archive: --remote= --exec= -- Hannes ``` ## Cornelius Weig, 2017-01-24 08:14 Subject: Re: [PATCH 7/7] completion: recognize more long-options Message-ID: <967937ff-e5ff-2515-2f50-80a96683c068@tngtech.com> URL: https://gitlist.dev/e/967937ff-e5ff-2515-2f50-80a96683c068%40tngtech.com In-Reply-To: <74ecd09c-55da-3858-5187-52c286a6bf62@kdbg.org> ``` On 01/24/2017 08:15 AM, Johannes Sixt wrote: > If at all possible, please use your real email address as the From > address. It is pointless to hide behind a fake address because as Git > contributor you will have to reveal your identity anyway. These are both real addresses, but for send-mail I would not want to use my work account. I hope this is not a problem. > Please study item (5) "Sign your work" in > Documentation/SubmittingPatches and sign off your work. I followed the recommendations to submitting work, and in the first round signing is discouraged. > AFAIR, it was a deliberate decision that potentially destructive command > line options are not included in command completions. In the list given, > I find these: > >> - reset: --merge --mixed --hard --soft --patch --keep My bad, I only added --keep, which should be fine. As to these options >> - apply: --unsafe-paths >> - rm: --force let's wait for further comments, but I won't cling to it. > Additionally, these options are only for internal purposes, but I may be > wrong: > >> - archive: --remote= --exec= These may in fact be too exotic and just clutter the command line. Best they are removed. -- Cornelius ``` ## Junio C Hamano, 2017-01-24 23:24 Subject: Re: [PATCH 7/7] completion: recognize more long-options Message-ID: URL: https://gitlist.dev/e/xmqqd1fcca8n.fsf%40gitster.mtv.corp.google.com In-Reply-To: <967937ff-e5ff-2515-2f50-80a96683c068@tngtech.com> ``` Cornelius Weig writes: >> Please study item (5) "Sign your work" in >> Documentation/SubmittingPatches and sign off your work. > > I followed the recommendations to submitting work, and in the first > round signing is discouraged. Just this point. You found a bug in our documentation if that is the case; it should not be giving that impression to you. ``` ## Cornelius Weig, 2017-01-24 23:33 Subject: Re: [PATCH 7/7] completion: recognize more long-options Message-ID: URL: https://gitlist.dev/e/a921bc92-4f20-3284-6577-344470a60c6f%40tngtech.com In-Reply-To: ``` On 01/25/2017 12:24 AM, Junio C Hamano wrote: > Cornelius Weig writes: > >>> Please study item (5) "Sign your work" in >>> Documentation/SubmittingPatches and sign off your work. >> >> I followed the recommendations to submitting work, and in the first >> round signing is discouraged. > > Just this point. You found a bug in our documentation if that is > the case; it should not be giving that impression to you. > Well, I am referring to par. (4) of Documentation/SubmittingPatches (emphasis mine): <<<<<<<<<<<<<< *Do not PGP sign your patch, at least for now*. Most likely, your maintainer or other people on the list would not have your PGP key and would not bother obtaining it anyway. Your patch is not judged by who you are; a good patch from an unknown origin has a far better chance of being accepted than a patch from a known, respected origin that is done poorly or does incorrect things. >>>>>>>>>>>>>> If first submissions should be signed as well, then I find this quite misleading. ``` ## Stefan Beller, 2017-01-24 23:43 Subject: Re: [PATCH 7/7] completion: recognize more long-options Message-ID: URL: https://gitlist.dev/e/CAGZ79ka0PSb9L71tkiacZS%2BFH%3DYbUBrQr6a5UQu7ochpihRqEQ%40mail.gmail.com In-Reply-To: ``` On Tue, Jan 24, 2017 at 3:33 PM, Cornelius Weig wrote: > On 01/25/2017 12:24 AM, Junio C Hamano wrote: >> Cornelius Weig writes: >> >>>> Please study item (5) "Sign your work" in >>>> Documentation/SubmittingPatches and sign off your work. >>> >>> I followed the recommendations to submitting work, and in the first >>> round signing is discouraged. >> >> Just this point. You found a bug in our documentation if that is >> the case; it should not be giving that impression to you. >> > > Well, I am referring to par. (4) of Documentation/SubmittingPatches > (emphasis mine): > > <<<<<<<<<<<<<< > *Do not PGP sign your patch, at least for now*. Most likely, your > maintainer or other people on the list would not have your PGP > key and would not bother obtaining it anyway. Your patch is not > judged by who you are; a good patch from an unknown origin has a > far better chance of being accepted than a patch from a known, > respected origin that is done poorly or does incorrect things. >>>>>>>>>>>>>>> > > If first submissions should be signed as well, then I find this quite > misleading. > Please read on; While this part addresses PGP signing, which is discouraged at any round, later on we talk about another type of signing. (not cryptographic strong signing, but signing the intent;) the DCO in the commit message. So no PGP signing (in any round of the patch). But DCO signed (in anything that you deem useful for the project and that you are allowed to contribute) ``` ## Cornelius Weig, 2017-01-25 00:11 Subject: Re: [PATCH 7/7] completion: recognize more long-options Message-ID: <923cd4e4-5c9c-4eaf-0fea-6deff6875b88@tngtech.com> URL: https://gitlist.dev/e/923cd4e4-5c9c-4eaf-0fea-6deff6875b88%40tngtech.com In-Reply-To: ``` On 01/25/2017 12:43 AM, Stefan Beller wrote: > On Tue, Jan 24, 2017 at 3:33 PM, Cornelius Weig > wrote: >> On 01/25/2017 12:24 AM, Junio C Hamano wrote: >>> Cornelius Weig writes: >>> >>>>> Please study item (5) "Sign your work" in >>>>> Documentation/SubmittingPatches and sign off your work. >>>> >>>> I followed the recommendations to submitting work, and in the first >>>> round signing is discouraged. >>> >>> Just this point. You found a bug in our documentation if that is >>> the case; it should not be giving that impression to you. >>> >> >> Well, I am referring to par. (4) of Documentation/SubmittingPatches >> (emphasis mine): >> >> <<<<<<<<<<<<<< >> *Do not PGP sign your patch, at least for now*. Most likely, your >> maintainer or other people on the list would not have your PGP >> key and would not bother obtaining it anyway. Your patch is not >> judged by who you are; a good patch from an unknown origin has a >> far better chance of being accepted than a patch from a known, >> respected origin that is done poorly or does incorrect things. >> <<<<<<<<<<<<<< >> >> If first submissions should be signed as well, then I find this quite >> misleading. >> > > Please read on; While this part addresses PGP signing, > which is discouraged at any round, > later on we talk about another type of signing. > (not cryptographic strong signing, but signing the intent;) > the DCO in the commit message. > > So no PGP signing (in any round of the patch). > > But DCO signed (in anything that you deem useful for the > project and that you are allowed to contribute) > Right, it's crystal clear now. What confused me was the combination of > Do not PGP sign your patch, at least *for now*. (...) and then the section with heading > (5) *Sign* your work So I didn't even bother to read (5) because I deemed it irrelevant. I think if it had said `(5) *Certify* your work` this would not have happened. ``` ## Stefan Beller, 2017-01-25 00:21 Subject: Message-ID: <20170125002116.22111-1-sbeller@google.com> URL: https://gitlist.dev/e/20170125002116.22111-1-sbeller%40google.com In-Reply-To: <923cd4e4-5c9c-4eaf-0fea-6deff6875b88@tngtech.com> ``` > >> Do not PGP sign your patch, at least *for now*. (...) > And maybe these 2 small words are the bug in the documentation? Shall we drop the "at least for now" part, like so: ---8<--- From 2c4fe0e67451892186ff6257b20c53e088c9ec67 Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Tue, 24 Jan 2017 16:19:13 -0800 Subject: [PATCH] SubmittingPatches: drop temporal reference for PGP signing Signed-off-by: Stefan Beller --- Documentation/SubmittingPatches | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index 08352deaae..28da4ad2d4 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches @@ -216,12 +216,12 @@ that it will be postponed. Exception: If your mailer is mangling patches then someone may ask you to re-send them using MIME, that is OK. -Do not PGP sign your patch, at least for now. Most likely, your -maintainer or other people on the list would not have your PGP -key and would not bother obtaining it anyway. Your patch is not -judged by who you are; a good patch from an unknown origin has a -far better chance of being accepted than a patch from a known, -respected origin that is done poorly or does incorrect things. +Do not PGP sign your patch. Most likely, your maintainer or other +people on the list would not have your PGP key and would not bother +obtaining it anyway. Your patch is not judged by who you are; a good +patch from an unknown origin has a far better chance of being accepted +than a patch from a known, respected origin that is done poorly or +does incorrect things. If you really really really really want to do a PGP signed patch, format it as "multipart/signed", not a text/plain message -- 2.11.0.495.g04f60290a0.dirty ``` ## Cornelius Weig, 2017-01-25 00:43 Subject: Re: Message-ID: URL: https://gitlist.dev/e/ec2a198e-edf5-68a1-523c-12a9d1333c58%40tngtech.com In-Reply-To: <20170125002116.22111-1-sbeller@google.com> ``` On 01/25/2017 01:21 AM, Stefan Beller wrote: >> >>> Do not PGP sign your patch, at least *for now*. (...) >> > > And maybe these 2 small words are the bug in the documentation? > Shall we drop the "at least for now" part, like so: > > ---8<--- > From 2c4fe0e67451892186ff6257b20c53e088c9ec67 Mon Sep 17 00:00:00 2001 > From: Stefan Beller > Date: Tue, 24 Jan 2017 16:19:13 -0800 > Subject: [PATCH] SubmittingPatches: drop temporal reference for PGP signing > > Signed-off-by: Stefan Beller > --- > Documentation/SubmittingPatches | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches > index 08352deaae..28da4ad2d4 100644 > --- a/Documentation/SubmittingPatches > +++ b/Documentation/SubmittingPatches > @@ -216,12 +216,12 @@ that it will be postponed. > Exception: If your mailer is mangling patches then someone may ask > you to re-send them using MIME, that is OK. > > -Do not PGP sign your patch, at least for now. Most likely, your > -maintainer or other people on the list would not have your PGP > -key and would not bother obtaining it anyway. Your patch is not > -judged by who you are; a good patch from an unknown origin has a > -far better chance of being accepted than a patch from a known, > -respected origin that is done poorly or does incorrect things. > +Do not PGP sign your patch. Most likely, your maintainer or other > +people on the list would not have your PGP key and would not bother > +obtaining it anyway. Your patch is not judged by who you are; a good > +patch from an unknown origin has a far better chance of being accepted > +than a patch from a known, respected origin that is done poorly or > +does incorrect things. > > If you really really really really want to do a PGP signed > patch, format it as "multipart/signed", not a text/plain message > It definitely is an improvement. Though it would still leave me puzzled when finding a section about signing just below. Is changing heading (5) too big a change? Like so: diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index 08352de..71898dc 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches @@ -246,7 +246,7 @@ patch. *2* The mailing list: git@vger.kernel.org -(5) Sign your work +(5) Certify your work by signing off. To improve tracking of who did what, we've borrowed the "sign-off" procedure from the Linux kernel project on patches ``` ## Stefan Beller, 2017-01-25 00:52 Subject: Re: Message-ID: URL: https://gitlist.dev/e/CAGZ79ka2b4BxYXEou6QrvxwbTjTdd7BRqhA1ZEw8e-gjB3cDDQ%40mail.gmail.com In-Reply-To: ``` On Tue, Jan 24, 2017 at 4:43 PM, Cornelius Weig wrote: > -(5) Sign your work > +(5) Certify your work by signing off. That sounds better than what I proposed. Thanks, Stefan ``` ## Linus Torvalds, 2017-01-25 00:54 Subject: Re: Message-ID: URL: https://gitlist.dev/e/CA%2B55aFx_W500Ct6HuG18owG37FviirjsrJ_4zZpRcDD-0tmpFg%40mail.gmail.com In-Reply-To: <20170125002116.22111-1-sbeller@google.com> ``` On Tue, Jan 24, 2017 at 4:21 PM, Stefan Beller wrote: > > +Do not PGP sign your patch. Most likely, your maintainer or other > +people on the list would not have your PGP key and would not bother > +obtaining it anyway. I think even that could be further simplified - by just removing all comments about pgp email Because it's not that the PGP keys would be hard to get, it's that PGP-signed email is an abject failure, and nobody sane does it. Google for "phil zimmerman doesn't use pgp email". It's dead. So I'm not sure it's worth mentioning at all. You might as well talk about how you shouldn't use EBCDIC encoding for your patches, or about why git assumes that an email address has an '@' sign in it, instead of being an UUCP bang path address. Linus ``` ## Eric Wong, 2017-01-25 01:32 Subject: Re: Message-ID: <20170125013233.GA16463@starla> URL: https://gitlist.dev/e/20170125013233.GA16463%40starla In-Reply-To: ``` Linus Torvalds wrote: > On Tue, Jan 24, 2017 at 4:21 PM, Stefan Beller wrote: > > > > +Do not PGP sign your patch. Most likely, your maintainer or other > > +people on the list would not have your PGP key and would not bother > > +obtaining it anyway. > > I think even that could be further simplified - by just removing all > comments about pgp email > > Because it's not that the PGP keys would be hard to get, it's that > PGP-signed email is an abject failure, and nobody sane does it. > > Google for "phil zimmerman doesn't use pgp email". > > It's dead. So I'm not sure it's worth mentioning at all. I disagree, we still see it, and Debian still advocates it. In fact, we may also want to mention S/MIME in the same breath: https://public-inbox.org/git/20170110004031.57985-2-hansenr@google.com/ Richard's more recent mails seem to indicate he's reformed :) ``` ## Philip Oakley, 2017-01-25 06:55 Subject: Re: SubmittingPatches: drop temporal reference for PGP signing Message-ID: <33E354BCDB9A4192B69B9B399381659E@PhilipOakley> URL: https://gitlist.dev/e/33E354BCDB9A4192B69B9B399381659E%40PhilipOakley In-Reply-To: <20170125002116.22111-1-sbeller@google.com> ``` From: "Stefan Beller" >> >>> Do not PGP sign your patch, at least *for now*. (...) >> > > And maybe these 2 small words are the bug in the documentation? > Shall we drop the "at least for now" part, like so: > > ---8<--- > From 2c4fe0e67451892186ff6257b20c53e088c9ec67 Mon Sep 17 00:00:00 2001 > From: Stefan Beller > Date: Tue, 24 Jan 2017 16:19:13 -0800 > Subject: [PATCH] SubmittingPatches: drop temporal reference for PGP > signing > > Signed-off-by: Stefan Beller > --- > Documentation/SubmittingPatches | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/Documentation/SubmittingPatches > b/Documentation/SubmittingPatches > index 08352deaae..28da4ad2d4 100644 > --- a/Documentation/SubmittingPatches > +++ b/Documentation/SubmittingPatches > @@ -216,12 +216,12 @@ that it will be postponed. > Exception: If your mailer is mangling patches then someone may ask > you to re-send them using MIME, that is OK. > > -Do not PGP sign your patch, at least for now. Most likely, your > -maintainer or other people on the list would not have your PGP > -key and would not bother obtaining it anyway. Your patch is not > -judged by who you are; a good patch from an unknown origin has a > -far better chance of being accepted than a patch from a known, > -respected origin that is done poorly or does incorrect things. > +Do not PGP sign your patch. Most likely, your maintainer or other > +people on the list would not have your PGP key and would not bother > +obtaining it anyway. Your patch is not judged by who you are; a good > +patch from an unknown origin has a far better chance of being accepted > +than a patch from a known, respected origin that is done poorly or > +does incorrect things. Wouldn't this also benefit from a forward reference to the section 5 on the DOC signining? This would avoid Cornelius's case where he felt that section 5 no longer applied. > If you really really really really want to do a PGP signed > patch, format it as "multipart/signed", not a text/plain message > -- > 2.11.0.495.g04f60290a0.dirty -- Philip ``` ## Stefan Beller, 2017-01-25 22:38 Subject: Re: SubmittingPatches: drop temporal reference for PGP signing Message-ID: URL: https://gitlist.dev/e/CAGZ79kaRdtKD7DNJRWXsyg07GbTM4OsKUmHHcFczEMJA1YK2KA%40mail.gmail.com In-Reply-To: <33E354BCDB9A4192B69B9B399381659E@PhilipOakley> ``` On Tue, Jan 24, 2017 at 10:54 PM, Philip Oakley wrote: >> -Do not PGP sign your patch, at least for now. Most likely, your >> -maintainer or other people on the list would not have your PGP >> -key and would not bother obtaining it anyway. Your patch is not >> -judged by who you are; a good patch from an unknown origin has a >> -far better chance of being accepted than a patch from a known, >> -respected origin that is done poorly or does incorrect things. >> +Do not PGP sign your patch. Most likely, your maintainer or other >> +people on the list would not have your PGP key and would not bother >> +obtaining it anyway. Your patch is not judged by who you are; a good >> +patch from an unknown origin has a far better chance of being accepted >> +than a patch from a known, respected origin that is done poorly or >> +does incorrect things. > > > Wouldn't this also benefit from a forward reference to the section 5 on the > DOC signining? This would avoid Cornelius's case where he felt that section > 5 no longer applied. Yeah I agree. My patch was not the best shot by far. ``` ## Cornelius Weig, 2017-01-26 13:30 Subject: Re: SubmittingPatches: drop temporal reference for PGP signing Message-ID: URL: https://gitlist.dev/e/baff65ba-1e98-d5a7-5b5a-a50a7fc723ee%40tngtech.com In-Reply-To: ``` > > Yeah I agree. My patch was not the best shot by far. > How about something along these lines? Does the forward reference break the main line of thought too severly? diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches index 08352de..c2b0cbe 100644 --- a/Documentation/SubmittingPatches +++ b/Documentation/SubmittingPatches @@ -216,12 +216,12 @@ that it will be postponed. Exception: If your mailer is mangling patches then someone may ask you to re-send them using MIME, that is OK. -Do not PGP sign your patch, at least for now. Most likely, your -maintainer or other people on the list would not have your PGP -key and would not bother obtaining it anyway. Your patch is not -judged by who you are; a good patch from an unknown origin has a -far better chance of being accepted than a patch from a known, -respected origin that is done poorly or does incorrect things. +Do not PGP sign your patch, but do sign-off your work as explained in (5). +Most likely, your maintainer or other people on the list would not have your +PGP key and would not bother obtaining it anyway. Your patch is not judged by +who you are; a good patch from an unknown origin has a far better chance of +being accepted than a patch from a known, respected origin that is done poorly +or does incorrect things. If you really really really really want to do a PGP signed patch, format it as "multipart/signed", not a text/plain message @@ -246,7 +246,7 @@ patch. *2* The mailing list: git@vger.kernel.org -(5) Sign your work +(5) Certify your work by signing off To improve tracking of who did what, we've borrowed the "sign-off" procedure from the Linux kernel project on patches ``` ## Stefan Beller, 2017-01-26 17:57 Subject: Re: SubmittingPatches: drop temporal reference for PGP signing Message-ID: URL: https://gitlist.dev/e/CAGZ79kYChm3Y4T%3DJk0-6cA3gEDpFDCHgXsP60pQ0afBDePWRXA%40mail.gmail.com In-Reply-To: ``` On Thu, Jan 26, 2017 at 5:30 AM, Cornelius Weig wrote: > >> >> Yeah I agree. My patch was not the best shot by far. >> > > How about something along these lines? Does the forward reference > break the main line of thought too severly? > > diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches > index 08352de..c2b0cbe 100644 > --- a/Documentation/SubmittingPatches > +++ b/Documentation/SubmittingPatches > @@ -216,12 +216,12 @@ that it will be postponed. > Exception: If your mailer is mangling patches then someone may ask > you to re-send them using MIME, that is OK. > > -Do not PGP sign your patch, at least for now. Most likely, your > -maintainer or other people on the list would not have your PGP > -key and would not bother obtaining it anyway. Your patch is not > -judged by who you are; a good patch from an unknown origin has a > -far better chance of being accepted than a patch from a known, > -respected origin that is done poorly or does incorrect things. > +Do not PGP sign your patch, but do sign-off your work as explained in (5). > +Most likely, your maintainer or other people on the list would not have your > +PGP key and would not bother obtaining it anyway. Your patch is not judged by > +who you are; a good patch from an unknown origin has a far better chance of > +being accepted than a patch from a known, respected origin that is done poorly > +or does incorrect things. > > If you really really really really want to do a PGP signed > patch, format it as "multipart/signed", not a text/plain message > @@ -246,7 +246,7 @@ patch. > *2* The mailing list: git@vger.kernel.org > > > -(5) Sign your work > +(5) Certify your work by signing off > > To improve tracking of who did what, we've borrowed the > "sign-off" procedure from the Linux kernel project on patches I like it. Thanks, Stefan ``` ## Junio C Hamano, 2017-01-26 18:18 Subject: Re: SubmittingPatches: drop temporal reference for PGP signing Message-ID: URL: https://gitlist.dev/e/xmqqpoj965yf.fsf%40gitster.mtv.corp.google.com In-Reply-To: ``` Cornelius Weig writes: > How about something along these lines? Does the forward reference > break the main line of thought too severly? I find it a bit distracting for those who know PGP signing has nothing to do with signing off your patch, but I think that is OK because they are not the primary target audience of this part of the document. I however am more worried that it may be misleading to mention these two in the same sentence. Those who skim these paragraphs without knowing the difference between the two may get a false impression that these two may somehow be related because they are mentioned in the same sentence. The retitling of section (5) you did, without any other change, might be sufficient. It may also help to be even more explicit in the updated title, i.e. s/by signing off/by adding Signed-off-by:/ Thanks. > diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches > index 08352de..c2b0cbe 100644 > --- a/Documentation/SubmittingPatches > +++ b/Documentation/SubmittingPatches > @@ -216,12 +216,12 @@ that it will be postponed. > Exception: If your mailer is mangling patches then someone may ask > you to re-send them using MIME, that is OK. > > -Do not PGP sign your patch, at least for now. Most likely, your > -maintainer or other people on the list would not have your PGP > -key and would not bother obtaining it anyway. Your patch is not > -judged by who you are; a good patch from an unknown origin has a > -far better chance of being accepted than a patch from a known, > -respected origin that is done poorly or does incorrect things. > +Do not PGP sign your patch, but do sign-off your work as explained in (5). > +Most likely, your maintainer or other people on the list would not have your > +PGP key and would not bother obtaining it anyway. Your patch is not judged by > +who you are; a good patch from an unknown origin has a far better chance of > +being accepted than a patch from a known, respected origin that is done poorly > +or does incorrect things. > > If you really really really really want to do a PGP signed > patch, format it as "multipart/signed", not a text/plain message > @@ -246,7 +246,7 @@ patch. > *2* The mailing list: git@vger.kernel.org > > > -(5) Sign your work > +(5) Certify your work by signing off > > To improve tracking of who did what, we've borrowed the > "sign-off" procedure from the Linux kernel project on patches ``` ## Philip Oakley, 2017-01-26 20:59 Subject: Re: SubmittingPatches: drop temporal reference for PGP signing Message-ID: <4B89512D54614F09817EA9901A8B625D@PhilipOakley> URL: https://gitlist.dev/e/4B89512D54614F09817EA9901A8B625D%40PhilipOakley In-Reply-To: ``` From: "Junio C Hamano" > Cornelius Weig writes: > >> How about something along these lines? Does the forward reference >> break the main line of thought too severly? > > I find it a bit distracting for those who know PGP signing has > nothing to do with signing off your patch, but I think that is OK > because they are not the primary target audience of this part of the > document. Agreed. I this case the target audience was those weren't aware of that. > > I however am more worried that it may be misleading to mention these > two in the same sentence. Those who skim these paragraphs without > knowing the difference between the two may get a false impression > that these two may somehow be related because they are mentioned in > the same sentence. > > The retitling of section (5) you did, without any other change, > might be sufficient. It may also help to be even more explicit in > the updated title, i.e. s/by signing off/by adding Signed-off-by:/ Maybe even s/by signing off/by adding your Signed-off-by:/ to be sure that the reader knows that it is _their certification_ that is being sought. Even if it does double up on the 'your'. > > Thanks. > >> diff --git a/Documentation/SubmittingPatches >> b/Documentation/SubmittingPatches >> index 08352de..c2b0cbe 100644 >> --- a/Documentation/SubmittingPatches >> +++ b/Documentation/SubmittingPatches >> @@ -216,12 +216,12 @@ that it will be postponed. >> Exception: If your mailer is mangling patches then someone may ask >> you to re-send them using MIME, that is OK. >> >> -Do not PGP sign your patch, at least for now. Most likely, your >> -maintainer or other people on the list would not have your PGP >> -key and would not bother obtaining it anyway. Your patch is not >> -judged by who you are; a good patch from an unknown origin has a >> -far better chance of being accepted than a patch from a known, >> -respected origin that is done poorly or does incorrect things. >> +Do not PGP sign your patch, but do sign-off your work as explained in >> (5). >> +Most likely, your maintainer or other people on the list would not have >> your >> +PGP key and would not bother obtaining it anyway. Your patch is not >> judged by >> +who you are; a good patch from an unknown origin has a far better chance >> of >> +being accepted than a patch from a known, respected origin that is done >> poorly >> +or does incorrect things. >> >> If you really really really really want to do a PGP signed >> patch, format it as "multipart/signed", not a text/plain message >> @@ -246,7 +246,7 @@ patch. >> *2* The mailing list: git@vger.kernel.org >> >> >> -(5) Sign your work >> +(5) Certify your work by signing off >> >> To improve tracking of who did what, we've borrowed the >> "sign-off" procedure from the Linux kernel project on patches > ``` ## Cornelius Weig, 2017-01-27 10:49 Subject: Re: SubmittingPatches: drop temporal reference for PGP signing Message-ID: <60e9abdf-cc37-33d8-e7eb-8a3370ffe1cc@tngtech.com> URL: https://gitlist.dev/e/60e9abdf-cc37-33d8-e7eb-8a3370ffe1cc%40tngtech.com In-Reply-To: <4B89512D54614F09817EA9901A8B625D@PhilipOakley> ``` On 01/26/2017 09:58 PM, Philip Oakley wrote: > From: "Junio C Hamano" >> Cornelius Weig writes: >> >>> How about something along these lines? Does the forward reference >>> break the main line of thought too severly? >> >> I find it a bit distracting for those who know PGP signing has >> nothing to do with signing off your patch, but I think that is OK >> because they are not the primary target audience of this part of the >> document. > > Agreed. I this case the target audience was those weren't aware of that. Yes, maybe the forward reference does give a wrong hint about a connection between sign-off and pgp-signing. However, I would still vote for the following change suggested by sbeller@google.com: -Do not PGP sign your patch, -at least for now-. Most likely, your (...) +Do not PGP sign your patch. Most likely, your maintainer or other (...) > > Maybe even s/by signing off/by adding your Signed-off-by:/ to be sure > that the reader knows that it is _their certification_ that is being > sought. Even if it does double up on the 'your'. > I don't think doubling on the 'your' will make the heading clearer. The main intention of this change is to avoid mixups with pgp-signing and that would IMHO not improve by that. Besides, I find the colon in the heading a bit awkward. Is the following version as expressive as with the colon? -(5) Sign your work +(5) Certify your work by adding Signed-off-by ``` ## Junio C Hamano, 2017-01-27 17:43 Subject: Re: SubmittingPatches: drop temporal reference for PGP signing Message-ID: URL: https://gitlist.dev/e/xmqq60l01jr9.fsf%40gitster.mtv.corp.google.com In-Reply-To: <60e9abdf-cc37-33d8-e7eb-8a3370ffe1cc@tngtech.com> ``` Cornelius Weig writes: > -Do not PGP sign your patch, -at least for now-. Most likely, your (...) > +Do not PGP sign your patch. Most likely, your maintainer or other (...) It has been quite a while since we wrote that "at least for now", so it probably makes sense to drop it. >> Maybe even s/by signing off/by adding your Signed-off-by:/ to be sure >> that the reader knows that it is _their certification_ that is being >> sought. Even if it does double up on the 'your'. > > I don't think doubling on the 'your' will make the heading clearer. The > main intention of this change is to avoid mixups with pgp-signing and > that would IMHO not improve by that. > Besides, I find the colon in the heading a bit awkward. Is the following > version as expressive as with the colon? > > -(5) Sign your work > +(5) Certify your work by adding Signed-off-by I am leaning towards agreeing with Philip, and my gut feeling says the presense of the colon, would help a lot. It would visually click to readers what we are talking about. At the same time, I think ending a section header with a colon would be seen as "awkward". I would have written it more like this to avoid it: ... by adding "Signed-off-by: " line I personally do not mind having "your" there, either, but I can see that a section header wants to be kept shorter. ``` ## cornelius.weig@tngtech.com, 2017-01-27 21:17 Subject: [PATCH v2 7/7] completion: recognize more long-options Message-ID: <20170127211703.24910-2-cornelius.weig@tngtech.com> URL: https://gitlist.dev/e/20170127211703.24910-2-cornelius.weig%40tngtech.com In-Reply-To: <20170127211703.24910-1-cornelius.weig@tngtech.com> ``` From: Cornelius Weig Recognize several new long-options for bash completion in the following commands: - apply: --recount --directory= - archive: --output - branch: --column --no-column --sort= --points-at - clone: --no-single-branch --shallow-submodules - commit: --patch --short --date --allow-empty - describe: --first-parent - fetch, pull: --unshallow --update-shallow - fsck: --name-objects - grep: --break --heading --show-function --function-context --untracked --no-index - mergetool: --prompt --no-prompt - reset: --keep - revert: --strategy= --strategy-option= - rm: --force - shortlog: --email - tag: --merged --no-merged --create-reflog Signed-off-by: Cornelius Weig Helped-by: Johannes Sixt --- contrib/completion/git-completion.bash | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 0e09519..933bb6e 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -936,6 +936,7 @@ _git_apply () --apply --no-add --exclude= --ignore-whitespace --ignore-space-change --whitespace= --inaccurate-eof --verbose + --recount --directory= " return esac @@ -974,7 +975,7 @@ _git_archive () --*) __gitcomp " --format= --list --verbose - --prefix= --remote= --exec= + --prefix= --remote= --exec= --output " return ;; @@ -1029,6 +1030,7 @@ _git_branch () --track --no-track --contains --merged --no-merged --set-upstream-to= --edit-description --list --unset-upstream --delete --move --remotes + --column --no-column --sort= --points-at " ;; *) @@ -1142,6 +1144,8 @@ _git_clone () --single-branch --branch --recurse-submodules + --no-single-branch + --shallow-submodules " return ;; @@ -1183,6 +1187,7 @@ _git_commit () --reset-author --file= --message= --template= --cleanup= --untracked-files --untracked-files= --verbose --quiet --fixup= --squash= + --patch --short --date --allow-empty " return esac @@ -1201,7 +1206,7 @@ _git_describe () --*) __gitcomp " --all --tags --contains --abbrev= --candidates= - --exact-match --debug --long --match --always + --exact-match --debug --long --match --always --first-parent " return esac @@ -1284,6 +1289,7 @@ __git_fetch_recurse_submodules="yes on-demand no" __git_fetch_options=" --quiet --verbose --append --upload-pack --force --keep --depth= --tags --no-tags --all --prune --dry-run --recurse-submodules= + --unshallow --update-shallow " _git_fetch () @@ -1333,7 +1339,7 @@ _git_fsck () --*) __gitcomp " --tags --root --unreachable --cache --no-reflogs --full - --strict --verbose --lost-found + --strict --verbose --lost-found --name-objects " return ;; @@ -1377,6 +1383,8 @@ _git_grep () --max-depth --count --and --or --not --all-match + --break --heading --show-function --function-context + --untracked --no-index " return ;; @@ -1576,7 +1584,7 @@ _git_mergetool () return ;; --*) - __gitcomp "--tool=" + __gitcomp "--tool= --prompt --no-prompt" return ;; esac @@ -2456,7 +2464,7 @@ _git_reset () case "$cur" in --*) - __gitcomp "--merge --mixed --hard --soft --patch" + __gitcomp "--merge --mixed --hard --soft --patch --keep" return ;; esac @@ -2472,7 +2480,10 @@ _git_revert () fi case "$cur" in --*) - __gitcomp "--edit --mainline --no-edit --no-commit --signoff" + __gitcomp " + --edit --mainline --no-edit --no-commit --signoff + --strategy= --strategy-option= + " return ;; esac @@ -2483,7 +2494,7 @@ _git_rm () { case "$cur" in --*) - __gitcomp "--cached --dry-run --ignore-unmatch --quiet" + __gitcomp "--cached --dry-run --ignore-unmatch --quiet --force" return ;; esac @@ -2500,7 +2511,7 @@ _git_shortlog () __gitcomp " $__git_log_common_options $__git_log_shortlog_options - --numbered --summary + --numbered --summary --email " return ;; @@ -2778,8 +2789,8 @@ _git_tag () --*) __gitcomp " --list --delete --verify --annotate --message --file - --sign --cleanup --local-user --force --column --sort - --contains --points-at + --sign --cleanup --local-user --force --column --sort= + --contains --points-at --merged --no-merged --create-reflog " ;; esac -- 2.10.2 ``` ## cornelius.weig@tngtech.com, 2017-01-27 21:17 Subject: [PATCH v2 0/7] completion: recognize more long-options Message-ID: <20170127211703.24910-1-cornelius.weig@tngtech.com> URL: https://gitlist.dev/e/20170127211703.24910-1-cornelius.weig%40tngtech.com In-Reply-To: <74ecd09c-55da-3858-5187-52c286a6bf62@kdbg.org> ``` From: Cornelius Weig This revision addresses Johannes' concerns. Changes wrt v1: - fixed the commit message: two of the "dangerous" options erroneously ended up in the commit message. These options were already in the list of auto-completable options. - removed the possibly dangerous option '--unsafe-paths' from git-apply. - added my sign-off Patches 1-6 are not resent, because they have not changed (other than my added sign-off). Also, I added further people to CC, because nobody actually has looked at the code yet. Cornelius Weig (7): completion: recognize more long-options contrib/completion/git-completion.bash | 132 +++++++++++++++++++++++++++------ 1 file changed, 110 insertions(+), 22 deletions(-) -- 2.10.2 ``` ## SZEDER Gábor, 2017-01-31 22:17 Subject: Re: [PATCH v2 7/7] completion: recognize more long-options Message-ID: URL: https://gitlist.dev/e/CAM0VKj%3DEin4yrKG2aZnN7JU80ctZBQromcR6BEu-TyMLenLFCg%40mail.gmail.com In-Reply-To: <20170127211703.24910-2-cornelius.weig@tngtech.com> ``` On Fri, Jan 27, 2017 at 10:17 PM, wrote: > From: Cornelius Weig > > Recognize several new long-options for bash completion in the following > commands: Adding more long options that git commands learn along the way is always an improvement. However, seeing "_several_ new long options" (or "some long options" in one of the other patches in the series) makes the reader wonder: are these the only new long options missing or are there more? If there are more, why only these are added? If there aren't any more missing long options left, then please say so, e.g. "Add all missing long options, except the potentially desctructive ones, for the following commands: ...." > - apply: --recount --directory= > - archive: --output > - branch: --column --no-column --sort= --points-at > - clone: --no-single-branch --shallow-submodules > - commit: --patch --short --date --allow-empty > - describe: --first-parent > - fetch, pull: --unshallow --update-shallow > - fsck: --name-objects > - grep: --break --heading --show-function --function-context > --untracked --no-index > - mergetool: --prompt --no-prompt > - reset: --keep > - revert: --strategy= --strategy-option= > - rm: --force '--force' is a potentially destructive option, too. > - shortlog: --email > - tag: --merged --no-merged --create-reflog > > Signed-off-by: Cornelius Weig > Helped-by: Johannes Sixt > --- > contrib/completion/git-completion.bash | 31 +++++++++++++++++++++---------- > 1 file changed, 21 insertions(+), 10 deletions(-) > > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > index 0e09519..933bb6e 100644 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -936,6 +936,7 @@ _git_apply () > --apply --no-add --exclude= > --ignore-whitespace --ignore-space-change > --whitespace= --inaccurate-eof --verbose > + --recount --directory= > " > return > esac > @@ -974,7 +975,7 @@ _git_archive () > --*) > __gitcomp " > --format= --list --verbose > - --prefix= --remote= --exec= > + --prefix= --remote= --exec= --output > " > return > ;; > @@ -1029,6 +1030,7 @@ _git_branch () > --track --no-track --contains --merged --no-merged > --set-upstream-to= --edit-description --list > --unset-upstream --delete --move --remotes > + --column --no-column --sort= --points-at > " > ;; > *) > @@ -1142,6 +1144,8 @@ _git_clone () > --single-branch > --branch > --recurse-submodules > + --no-single-branch > + --shallow-submodules > " > return > ;; > @@ -1183,6 +1187,7 @@ _git_commit () > --reset-author --file= --message= --template= > --cleanup= --untracked-files --untracked-files= > --verbose --quiet --fixup= --squash= > + --patch --short --date --allow-empty > " > return > esac > @@ -1201,7 +1206,7 @@ _git_describe () > --*) > __gitcomp " > --all --tags --contains --abbrev= --candidates= > - --exact-match --debug --long --match --always > + --exact-match --debug --long --match --always --first-parent > " > return > esac > @@ -1284,6 +1289,7 @@ __git_fetch_recurse_submodules="yes on-demand no" > __git_fetch_options=" > --quiet --verbose --append --upload-pack --force --keep --depth= > --tags --no-tags --all --prune --dry-run --recurse-submodules= > + --unshallow --update-shallow > " > > _git_fetch () > @@ -1333,7 +1339,7 @@ _git_fsck () > --*) > __gitcomp " > --tags --root --unreachable --cache --no-reflogs --full > - --strict --verbose --lost-found > + --strict --verbose --lost-found --name-objects > " > return > ;; > @@ -1377,6 +1383,8 @@ _git_grep () > --max-depth > --count > --and --or --not --all-match > + --break --heading --show-function --function-context > + --untracked --no-index > " > return > ;; > @@ -1576,7 +1584,7 @@ _git_mergetool () > return > ;; > --*) > - __gitcomp "--tool=" > + __gitcomp "--tool= --prompt --no-prompt" > return > ;; > esac > @@ -2456,7 +2464,7 @@ _git_reset () > > case "$cur" in > --*) > - __gitcomp "--merge --mixed --hard --soft --patch" > + __gitcomp "--merge --mixed --hard --soft --patch --keep" > return > ;; > esac > @@ -2472,7 +2480,10 @@ _git_revert () > fi > case "$cur" in > --*) > - __gitcomp "--edit --mainline --no-edit --no-commit --signoff" > + __gitcomp " > + --edit --mainline --no-edit --no-commit --signoff > + --strategy= --strategy-option= > + " > return > ;; > esac > @@ -2483,7 +2494,7 @@ _git_rm () > { > case "$cur" in > --*) > - __gitcomp "--cached --dry-run --ignore-unmatch --quiet" > + __gitcomp "--cached --dry-run --ignore-unmatch --quiet --force" > return > ;; > esac > @@ -2500,7 +2511,7 @@ _git_shortlog () > __gitcomp " > $__git_log_common_options > $__git_log_shortlog_options > - --numbered --summary > + --numbered --summary --email > " > return > ;; > @@ -2778,8 +2789,8 @@ _git_tag () > --*) > __gitcomp " > --list --delete --verify --annotate --message --file > - --sign --cleanup --local-user --force --column --sort > - --contains --points-at > + --sign --cleanup --local-user --force --column --sort= > + --contains --points-at --merged --no-merged --create-reflog > " > ;; > esac > -- > 2.10.2 > ``` ## SZEDER Gábor, 2017-01-31 22:42 Subject: Re: [PATCH 3/7] completion: improve bash completion for git-add Message-ID: <20170131224241.25138-1-szeder.dev@gmail.com> URL: https://gitlist.dev/e/20170131224241.25138-1-szeder.dev%40gmail.com In-Reply-To: <20170122225724.19360-1-cornelius.weig@tngtech.com> ``` > Add some long-options for git-add and improve path completion when the > --update flag is given. Please tell us in the commit message _how_ this improves path completion. > --- > contrib/completion/git-completion.bash | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > index 8329f09..652c7e2 100644 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -947,13 +947,17 @@ _git_add () > --*) > __gitcomp " > --interactive --refresh --patch --update --dry-run > - --ignore-errors --intent-to-add > + --ignore-errors --intent-to-add --force --edit --chmod= I almost started complaining that '--force' should be used with care, but then realized that for 'git add' it only means adding ignored files. So in this particular case '--force' is not destructive and we can offer it among other long options. Good. > " > return > esac > > - # XXX should we check for --update and --all options ? > - __git_complete_index_file "--others --modified --directory --no-empty-directory" > + local complete_opt="--others --modified --directory --no-empty-directory" > + if test -n "$(__git_find_on_cmdline "-u --update")" > + then > + complete_opt="--modified" > + fi > + __git_complete_index_file "$complete_opt" > } > > _git_archive () > -- > 2.10.2 ``` ## Cornelius Weig, 2017-02-01 16:49 Subject: Re: [PATCH v2 7/7] completion: recognize more long-options Message-ID: <2841d2de-32ad-eae8-6039-9251a40bb00e@tngtech.com> URL: https://gitlist.dev/e/2841d2de-32ad-eae8-6039-9251a40bb00e%40tngtech.com In-Reply-To: ``` Hi Gabor, thanks for taking a look at these commits. On 01/31/2017 11:17 PM, SZEDER Gábor wrote: > On Fri, Jan 27, 2017 at 10:17 PM, wrote: >> From: Cornelius Weig >> >> Recognize several new long-options for bash completion in the following >> commands: > > Adding more long options that git commands learn along the way is > always an improvement. However, seeing "_several_ new long options" > (or "some long options" in one of the other patches in the series) > makes the reader wonder: are these the only new long options missing > or are there more? If there are more, why only these are added? If > there aren't any more missing long options left, then please say so, > e.g. "Add all missing long options, except the potentially > desctructive ones, for the following commands: ...." Personally, I agree with you that > Adding more long options that git commands learn along the way is > always an improvement. However, people may start complaining that their terminal becomes too cluttered when doing a double-Tab. In my cover letter, I go to length about this. My assumption was that all options that are mentioned in the introduction of the command man-page should be important enough to have them in the completion list. I'll change my commit message accordingly. >> - rm: --force > > '--force' is a potentially destructive option, too. Thanks for spotting this. Btw, I haven't found that non-destructive options should not be eligible for completion. To avoid confusion about this in the future, I suggest to also change the documentation: index 933bb6e..96f1c7f 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -13,7 +13,7 @@ # *) git email aliases for git-send-email # *) tree paths within 'ref:path/to/file' expressions # *) file paths within current working directory and index -# *) common --long-options +# *) common non-destructive --long-options # # To use these routines: # I take it you have also looked at the code itself? Then I would gladly mention you as reviewer in my sign-off. ``` ## SZEDER Gábor, 2017-02-02 00:57 Subject: Re: [PATCH 2/7] completion: add subcommand completion for rerere Message-ID: <20170202005724.24754-1-szeder.dev@gmail.com> URL: https://gitlist.dev/e/20170202005724.24754-1-szeder.dev%40gmail.com In-Reply-To: <20170122225724.19360-3-cornelius.weig@tngtech.com> ``` > Managing recorded resolutions requires command-line usage of git-rerere. > Added subcommand completion for rerere and path completion for its > subcommand forget. > --- > contrib/completion/git-completion.bash | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > index c54a557..8329f09 100644 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -2401,6 +2401,17 @@ _git_replace () > __gitcomp_nl "$(__git_refs)" > } > > +_git_rerere () > +{ > + local subcommands="clear forget diff remaining status gc" > + local subcommand="$(__git_find_on_cmdline "$subcommands")" > + if test -z "$subcommand" > + then > + __gitcomp "$subcommands" > + return > + fi > +} > + > _git_reset () > { > __git_has_doubledash && return > -- > 2.10.2 You didn't add 'rerere' to the list of porcelain commands, i.e. it won't be listed after 'git '. I'm not saying it should be listed there, because I can't decide whether 'rerere' is considered porcelain or plumbing... Just wanted to make sure that this omission was intentional. ``` ## SZEDER Gábor, 2017-02-02 01:37 Subject: Re: [PATCH 6/7] completion: teach remote subcommands option completion Message-ID: <20170202013759.25789-1-szeder.dev@gmail.com> URL: https://gitlist.dev/e/20170202013759.25789-1-szeder.dev%40gmail.com In-Reply-To: <20170122225724.19360-7-cornelius.weig@tngtech.com> ``` > Git-remote needs to complete remote names, its subcommands, and options > thereof. In addition to the existing subcommand and remote name > completion, do also complete the options > > - add: --track --master --fetch --tags --no-tags --mirror= Oh, '--track' and '--master' are not even in the manpage or in 'git remote -h', I could only find them after looking at the source code... Good eyes :) > - set-url: --push --add --delete > - get-url: --push --all > - prune: --dry-run > --- > contrib/completion/git-completion.bash | 36 +++++++++++++++++++++++++++------- > 1 file changed, 29 insertions(+), 7 deletions(-) > > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > index e76cbd7..0e09519 100644 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -2384,24 +2384,46 @@ _git_config () > > _git_remote () > { > - local subcommands="add rename remove set-head set-branches set-url show prune update" > + local subcommands=" > + add rename remove set-head set-branches > + get-url set-url show prune update > + " > local subcommand="$(__git_find_on_cmdline "$subcommands")" > if [ -z "$subcommand" ]; then > - __gitcomp "$subcommands" > + case "$cur" in > + --*) > + __gitcomp "--verbose" > + ;; > + *) > + __gitcomp "$subcommands" > + ;; > + esac > return > fi > > - case "$subcommand" in > - rename|remove|set-url|show|prune) > - __gitcomp_nl "$(__git_remotes)" > + case "$subcommand,$cur" in > + add,--*) > + __gitcomp "--track --master --fetch --tags --no-tags --mirror=" > ;; > - set-head|set-branches) > + add,*) > + ;; > + set-head,*|set-branches,*) The 'set-head' subcommand has '--auto' and '--delete' options, and 'set-branches' has '--add'. > __git_complete_remote_or_refspec > ;; > - update) > + update,*) The 'update' subcommand has a '--prune' option. Otherwise it all looks good. > __gitcomp "$(__git_get_config_variables "remotes")" > ;; > + set-url,--*) > + __gitcomp "--push --add --delete" > + ;; > + get-url,--*) > + __gitcomp "--push --all" > + ;; > + prune,--*) > + __gitcomp "--dry-run" > + ;; > *) > + __gitcomp_nl "$(__git_remotes)" > ;; > esac > } > -- > 2.10.2 ``` ## SZEDER Gábor, 2017-02-02 01:40 Subject: Re: [PATCH 4/7] completion: teach ls-remote to complete options Message-ID: <20170202014014.25878-1-szeder.dev@gmail.com> URL: https://gitlist.dev/e/20170202014014.25878-1-szeder.dev%40gmail.com In-Reply-To: <20170122225724.19360-5-cornelius.weig@tngtech.com> ``` > ls-remote needs to complete remote names and its own options. And refnames, too. > In > addition to the existing remote name completions, do also complete > the options --heads, --tags, --refs, and --get-url. Why only these four options and not the other four? There are eight options in total here, so there is really no chance for cluttered terminals, and all eight are mentioned in the synopsis. > --- > contrib/completion/git-completion.bash | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > index 652c7e2..36fe439 100644 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -1449,6 +1449,12 @@ _git_ls_files () > > _git_ls_remote () > { > + case "$cur" in > + --*) > + __gitcomp "--heads --tags --refs --get-url" > + return > + ;; > + esac > __gitcomp_nl "$(__git_remotes)" > } > > -- > 2.10.2 ``` ## SZEDER Gábor, 2017-02-02 02:00 Subject: Re: [PATCH v2 7/7] completion: recognize more long-options Message-ID: URL: https://gitlist.dev/e/CAM0VKjkAnsT_LE4OZRkLPuiEZW88P7_OBbOw0XovHhLYfBhbwg%40mail.gmail.com In-Reply-To: <2841d2de-32ad-eae8-6039-9251a40bb00e@tngtech.com> ``` On Wed, Feb 1, 2017 at 5:49 PM, Cornelius Weig wrote: > Hi Gabor, > > thanks for taking a look at these commits. > > On 01/31/2017 11:17 PM, SZEDER Gábor wrote: >> On Fri, Jan 27, 2017 at 10:17 PM, wrote: >>> From: Cornelius Weig >>> >>> Recognize several new long-options for bash completion in the following >>> commands: >> >> Adding more long options that git commands learn along the way is >> always an improvement. However, seeing "_several_ new long options" >> (or "some long options" in one of the other patches in the series) >> makes the reader wonder: are these the only new long options missing >> or are there more? If there are more, why only these are added? If >> there aren't any more missing long options left, then please say so, >> e.g. "Add all missing long options, except the potentially >> desctructive ones, for the following commands: ...." > > Personally, I agree with you that >> Adding more long options that git commands learn along the way is >> always an improvement. > However, people may start complaining that their terminal becomes too > cluttered when doing a double-Tab. In my cover letter, I go to length > about this. My assumption was that all options that are mentioned in the > introduction of the command man-page should be important enough to have > them in the completion list. But that doesn't mean that the ones not mentioned in the synopsis section are not worth completing. The list of options listed by the completion script for several of these commands fits on a single line. The command with the most options among these is 'git pull', and even its options don't fill more than half of a 80x25 screen. I see no danger of people coming complaining. > I'll change my commit message accordingly. > >>> - rm: --force >> >> '--force' is a potentially destructive option, too. > > Thanks for spotting this. > > Btw, I haven't found that non-destructive options should not be eligible > for completion. To avoid confusion about this in the future, I suggest > to also change the documentation: > > index 933bb6e..96f1c7f 100644 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -13,7 +13,7 @@ > # *) git email aliases for git-send-email > # *) tree paths within 'ref:path/to/file' expressions > # *) file paths within current working directory and index > -# *) common --long-options > +# *) common non-destructive --long-options I don't mind such a change, but I don't think that list was ever meant to be comprehensive or decisive. It is definitely not the former, as it's missing several things that the completion script does support. OTOH, it talks about .git/remotes, which has been considered legacy for quite some years (though it's right, because the completion script still supports it). > I take it you have also looked at the code itself? Then I would gladly > mention you as reviewer in my sign-off. Yeah, most of the changes was rather straightforward, except the completion of 'git remote's subcommands' options, but that looks good, too. Gábor ``` ## Cornelius Weig, 2017-02-02 09:16 Subject: Re: [PATCH 2/7] completion: add subcommand completion for rerere Message-ID: <303e3381-ad15-c991-afc2-3a55706142f3@tngtech.com> URL: https://gitlist.dev/e/303e3381-ad15-c991-afc2-3a55706142f3%40tngtech.com In-Reply-To: <20170202005724.24754-1-szeder.dev@gmail.com> ``` On 02/02/2017 01:57 AM, SZEDER Gábor wrote: > You didn't add 'rerere' to the list of porcelain commands, i.e. it > won't be listed after 'git '. I'm not saying it should be > listed there, because I can't decide whether 'rerere' is considered > porcelain or plumbing... Just wanted to make sure that this omission > was intentional. Yes this is intentional. There is a number of plumbing commands that have command completion, but are not listed in 'git ' (e.g. ls-tree, ls-files, ls-remote, ...). Given that rerere will not be frequently invoked, I would not add it to the porcelain commands. ``` ## Cornelius Weig, 2017-02-02 09:40 Subject: Re: [PATCH 4/7] completion: teach ls-remote to complete options Message-ID: <603697da-c8e9-5644-e0f0-7ee265c069d8@tngtech.com> URL: https://gitlist.dev/e/603697da-c8e9-5644-e0f0-7ee265c069d8%40tngtech.com In-Reply-To: <20170202014014.25878-1-szeder.dev@gmail.com> ``` On 02/02/2017 02:40 AM, SZEDER Gábor wrote: > >> ls-remote needs to complete remote names and its own options. > > And refnames, too. Yes, right. However, do you think it is reasonable to complete remote refnames? I don't think so, because it would mean we would have to run ls-remote during completion -- and waiting for ls-remote could be quite lengthy. >> In >> addition to the existing remote name completions, do also complete >> the options --heads, --tags, --refs, and --get-url. > > Why only these four options and not the other four? > > There are eight options in total here, so there is really no chance > for cluttered terminals, and all eight are mentioned in the synopsis. My line of thought is the following: --quiet: does not print anything and is therefore only useful for scripting. Thus, there is no need to have it on the command line completion. --exit-code: has no visible effect and is only useful for scripting. --upload-pack: is really exotic. Nobody will ever use it without digging deep in the manuals. Therefore, I think it's unnecessary to have the option completable. --symref: Should probably be added, thanks. However, if you don't find my reasoning for omitting the three options above conclusive, I have no problem including them. ``` ## Cornelius Weig, 2017-02-02 10:29 Subject: Re: [PATCH 6/7] completion: teach remote subcommands option completion Message-ID: <16f6cc98-1467-27cf-1beb-3160358add5c@tngtech.com> URL: https://gitlist.dev/e/16f6cc98-1467-27cf-1beb-3160358add5c%40tngtech.com In-Reply-To: <20170202013759.25789-1-szeder.dev@gmail.com> ``` On 02/02/2017 02:37 AM, SZEDER Gábor wrote: > The 'set-head' subcommand has '--auto' and '--delete' options, and > 'set-branches' has '--add'. Oops. Thanks for spotting this.. >> __git_complete_remote_or_refspec >> ;; >> - update) >> + update,*) > > The 'update' subcommand has a '--prune' option. > ..and that. ``` ## Cornelius Weig, 2017-02-02 10:40 Subject: Re: [PATCH v2 7/7] completion: recognize more long-options Message-ID: <415570bf-7201-3b68-4c91-668741d50160@tngtech.com> URL: https://gitlist.dev/e/415570bf-7201-3b68-4c91-668741d50160%40tngtech.com In-Reply-To: ``` On 02/02/2017 03:00 AM, SZEDER Gábor wrote: >> Personally, I agree with you that >>> Adding more long options that git commands learn along the way is >>> always an improvement. >> However, people may start complaining that their terminal becomes too >> cluttered when doing a double-Tab. In my cover letter, I go to length >> about this. My assumption was that all options that are mentioned in the >> introduction of the command man-page should be important enough to have >> them in the completion list. > > But that doesn't mean that the ones not mentioned in the synopsis > section are not worth completing. Absolutely. What I meant is that at least the options from the synopsis should be contained in the set of completable options. >> Btw, I haven't found that non-destructive options should not be eligible >> for completion. To avoid confusion about this in the future, I suggest >> to also change the documentation: >> >> index 933bb6e..96f1c7f 100644 >> --- a/contrib/completion/git-completion.bash >> +++ b/contrib/completion/git-completion.bash >> @@ -13,7 +13,7 @@ >> # *) git email aliases for git-send-email >> # *) tree paths within 'ref:path/to/file' expressions >> # *) file paths within current working directory and index >> -# *) common --long-options >> +# *) common non-destructive --long-options > > I don't mind such a change, but I don't think that list was ever meant > to be comprehensive or decisive. It is definitely not the former, as > it's missing several things that the completion script does support. > OTOH, it talks about .git/remotes, which has been considered legacy > for quite some years (though it's right, because the completion script > still supports it). Then let's not do that change, because for some commands destructive long-options have been in the list of completed options for quite a while. Given that, the above change of the documentation, might stir up more confusion than it settles. ``` ## Junio C Hamano, 2017-02-02 16:57 Subject: Re: [PATCH 4/7] completion: teach ls-remote to complete options Message-ID: URL: https://gitlist.dev/e/xmqqwpd8leda.fsf%40gitster.mtv.corp.google.com In-Reply-To: <603697da-c8e9-5644-e0f0-7ee265c069d8@tngtech.com> ``` Cornelius Weig writes: > On 02/02/2017 02:40 AM, SZEDER Gábor wrote: >> >>> ls-remote needs to complete remote names and its own options. >> >> And refnames, too. > > Yes, right. However, do you think it is reasonable to complete remote > refnames? I don't think so, because it would mean we would have to run > ls-remote during completion -- and waiting for ls-remote could be quite > lengthy. ... and by the time the completion script knew what they are, we have all information necessary without actually having the user run the command ;-) That does sound backwards. I am however not sure what Szeder really meant by "refnames". For example, you may want to see that this $ git ls-remote origin mast peek into refs/remotes/origin/* and find those matching, i.e. most likely "master", and that can be done without talking to the remote side. It won't catch the case where the remote end added a new branch that also match, e.g. "mastiff", and it will actively harm the users by giving the impression that Git (collectively, even though from technical point of view, what the completion script does is not part of Git) told them that there is no such new branch they need to worry about. So probably even with the "peek local" optimization, I have a feeling that completion of refnames may not be such a good idea. ```