Re: [PATCH v7 12/13] completion: let git provide the completable command list
From: SZEDER Gábor <hidden>
Date: 2018-05-11 15:05:48
On Thu, May 10, 2018 at 10:46 AM, Nguyễn Thái Ngọc Duy [off-list ref] wrote:
Instead of maintaining a separate list of command classification, which often could go out of date, let's centralize the information back in git. While the function in git-completion.bash implies "list porcelain commands", that's not exactly what it does. It gets all commands (aka --list-cmds=main,others) then exclude certain non-porcelain ones. We could almost recreate this list two lists list-mainporcelain and others. The non-porcelain-but-included-anyway is added by the third category list-complete. list-complete does not recreate exactly the command list before this patch though. The following commands are not part of neither list-mainporcelain nor list-complete and as a result no longer completes: - annotate obsolete, discouraged to use - difftool-helper not an end user command - filter-branch not often used - get-tar-commit-id not often used - imap-send not often used - interpreter-trailers not for interactive use - lost-found obsolete - p4 too short and probably not often used (*) - peek-remote deprecated - svn same category as p4 (*) - tar-tree obsolete - verify-commit not often used
'git name-rev' is plumbing as well.
I think this commit should be split into two:
- first do the unequivocally beneficial thing and get rid of the
long, hard-coded command list in __git_list_porcelain_commands(),
while keeping its output unchanged,
- then do the arguable thing and change the list of commands.
Note that the current completion script incorrectly classifies filter-branch as porcelain and t9902 tests this behavior. We keep it this way in t9902 because this test does not really care which particular command is porcelain or plubmbing, they're just names. (*) to be fair, send-email command which is in the same foreignscminterface group as svn and p4 does get completion, just because it's used by git and kernel development. So maybe should include them. Signed-off-by: Nguyễn Thái Ngọc Duy <redacted> --- command-list.txt | 37 ++++---- contrib/completion/git-completion.bash | 117 ++++++------------------- t/t9902-completion.sh | 5 +- 3 files changed, 48 insertions(+), 111 deletions(-)
quoted hunk ↗ jump to hunk
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 4e724a5b76..908692ea52 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash@@ -835,18 +835,30 @@ __git_complete_strategy () } __git_commands () { - if test -n "${GIT_TESTING_COMMAND_COMPLETION:-}" - then - printf "%s" "${GIT_TESTING_COMMAND_COMPLETION}" - else - git --list-cmds=main,others - fi + case "$1" in + porcelain) + if test -n "$GIT_TESTING_PORCELAIN_COMMAND_LIST" + then + printf "%s" "$GIT_TESTING_PORCELAIN_COMMAND_LIST" + else + git --list-cmds=list-mainporcelain,others,list-complete + fi + ;; + all) + if test -n "$GIT_TESTING_ALL_COMMAND_LIST" + then + printf "%s" "$GIT_TESTING_ALL_COMMAND_LIST" + else + git --list-cmds=main,others + fi + ;; + esac } -__git_list_all_commands () +__git_list_commands ()
Please add comments before these functions to document their mandatory argument.
{
local i IFS=" "$'\n'
- for i in $(__git_commands)
+ for i in $(__git_commands $1)
do
case $i in
*--*) : helper pattern;;Is this loop to exclude helper commands with doubledash in their name still necessary?