Re: [PATCH 00/11] completion: general cleanups

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Apr 27, 2013, 13:07 UTC
Message-ID: <CAMP44s3h43S=A7+QEWuv0iefxbXyKZsiJ2QMdc-XiouoO3yYng@mail.gmail.com>
In-Reply-To: <CAMP44s1od7W0OufMhn2TCZTAo0aK9D+7VLzwVy7BSGNxAMuk6w@mail.gmail.com>

On Sat, Apr 27, 2013 at 7:36 AM, Felipe Contreras <felipe.contreras@gmail.com> wrote:

Show 65 quoted lines

> On Sat, Apr 27, 2013 at 6:33 AM, Manlio Perillo
> <manlio.perillo@gmail.com> wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> Il 27/04/2013 12:19, Felipe Contreras ha scritto:
>>> Hi,
>>>
>>> Basically while trying to understand the code for path completion, I found that
>>> a lot of code was duplicated, and for not much gain.
>>>
>>> I also noticed that doing 'git add file' doesn't add the trailing space as
>>> before. It's not clear if it should be possible to do that with -o filenames,
>>> but after all, what do -o filenames gives us? Nothing we can't do ourselves,
>>> apparently.
>>>
>>
>> No, you can not do it yourself, as far as I know.
>>
>> I added the `compopt -o filenames` on Junio request for something like
>> "It  would be nice if completion for real files would behave like
>> builtin bash completion", if I remember correctly.
>>
>> Try `git rm contrib/completion/<TAB>`, in the git reporitory.
>>
>> Using the new feature, bash will suggest:
>> "git-completion.bash  git-completion.tcsh  git-completion.zsh
>> git-prompt.sh"
>>
>> Old behaviour, instead, was to suggest:
>> "contrib/completion/git-completion.bash
>> contrib/completion/git-completion.zsh
>> contrib/completion/git-completion.tcsh  contrib/completion/git-prompt.sh"
>>
>> I tried several things, but I was unable to emulate Bash builtin file
>> completion, whithout having to use `compopt -o filenames`.
>
> I see. I'm not convinced it's such a great feature, but it would be
> nice to have.
>
> Anyway, 'compopt -o filenames +o nospace' should restore the old
> behavior to add a space after the completion.
>
>> As far as the "double slash" problem with the
>> __git_index_file_list_filter_bash function, please try
>> `git rm contrib<TAB>`.
>>
>> With current code, Bash will suggest:
>> "blameview/ diffall/ git-shell-commands/"
>>
>> If you remove the __git_index_file_list_filter_bash function and use
>> __git_index_file_list_filter_compat instead, Bash will suggest:
>>
>> "blameview// diffall// git-shell-commands//"
>>
>> I can confirm this on my system, and it was confirmed by another user.
>> It only happens when you use `compopt -o filenames`. I don't know if
>> this is a bug or a feature, but I can try to ask to Bash mailing list,
>> so that we can update the comment to make more clear why a separate
>> function was needed.
>
> I've managed to reproduce the issue. The slash doesn't appear in the
> completion, it appears on the list of completions.
>
> I'll see what I can think to fix the issues while still keep the code simple.

This should do the trick. No?

--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -262,16 +262,17 @@ __git_ls_files_helper ()
 #    If provided, only files within the specified directory are listed.
 #    Sub directories are never recursed.  Path must have a trailing
 #    slash.
+# 3. Compat mode; set to enable.
 __git_index_files ()
 {
-       local dir="$(__gitdir)" root="${2-.}" file
+       local dir="$(__gitdir)" root="${2-.}" file old="${3-}"

        if [ -d "$dir" ]; then
                __git_ls_files_helper "$root" "$1" |
                while read -r file; do
                        case "$file" in
-                       ?*/*) echo "${file%%/*}/" ;;
-                       *) echo "$file " ;;
+                       ?*/*) echo "${file%%/*}${old:+/}" ;;
+                       *) echo "${file}${old:+ }" ;;
                        esac
                done | sort | uniq
        fi
@@ -480,7 +481,7 @@ __git_complete_revlist_file ()
 # The exception is --committable, which finds the files appropriate commit.
 __git_complete_index_file ()
 {
-       local pfx="" cur_="$cur"
+       local pfx="" cur_="$cur" old

        case "$cur_" in
        ?*/*)
@@ -490,7 +491,8 @@ __git_complete_index_file ()
                ;;
        esac

-       __gitcomp_nl "$(__git_index_files "$1" "$pfx")" "$pfx" "$cur_" ""
+       compopt -o filenames +o nospace 2> /dev/null || old=1
+       __gitcomp_nl "$(__git_index_files "$1" "$pfx" "$old")" "$pfx" "$cur_" ""
 }

 __git_complete_file ()

-- 
Felipe Contreras