Thread (123 messages) 123 messages, 6 authors, 2018-05-13

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?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help