On Thu, May 10, 2018 at 10:46 AM, Nguyễn Thái Ngọc Duy
<[email protected]> 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 <[email protected]>
> ---
> command-list.txt | 37 ++++----
> contrib/completion/git-completion.bash | 117 ++++++-------------------
> t/t9902-completion.sh | 5 +-
> 3 files changed, 48 insertions(+), 111 deletions(-)
> 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?
