Re: [PATCH 00/11] completion: general cleanups
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
Il 27/04/2013 21:15, Felipe Contreras ha scritto:
> [...]
>>> @@ -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 ()
>>>
>>
>> I like the idea (but I have not tested it), however compopt is called
>> two times, for each completion.
>
> Why two times?
Ah, right; sorry.
I missed the fact that you are using __gitcomp_nl instead of my
__gitcomp_file.
>
>> Maybe we can test for `-o filenames` support when script is loaded,
>> where currently there is a Bash version check, and set a global variable?
>
> Yeah, that's the way bash-completion used to do it. But I wonder if we
> should be worrying about this at this point, even bash-completion
> dropped support for bash < 4 more than two years ago, and even debian
> stable is at 4.1.
>
I'm +0.
> [...]
Regards Manlio
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAlF8MVsACgkQscQJ24LbaUSY/wCgkq8CQeVGNpZFtchiLAKXYpxS
wsAAnR0abrQzA1jW+Do7CSuJOZVMRuJu
=zPgk
-END PGP SIGNATURE-
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/11] completion: general cleanups
On Sat, Apr 27, 2013 at 2:15 PM, Felipe Contreras
wrote:
> On Sat, Apr 27, 2013 at 10:40 AM, Manlio Perillo
> wrote:
>> -BEGIN PGP SIGNED MESSAGE-
>> Hash: SHA1
>>
>> Il 27/04/2013 15:07, Felipe Contreras ha scritto:
>>> [...]
>>> 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 ()
>>>
>>
>> I like the idea (but I have not tested it), however compopt is called
>> two times, for each completion.
>
> Why two times?
>
>> Maybe we can test for `-o filenames` support when script is loaded,
>> where currently there is a Bash version check, and set a global variable?
>
> Yeah, that's the way bash-completion used to do it. But I wonder if we
> should be worrying about this at this point, even bash-completion
> dropped support for bash < 4 more than two years ago, and even debian
> stable is at 4.1.
>
> http://anonscm.debian.org/gitweb/?p=bash-completion/bash-completion.git;a=commitdiff;h=f1b3be235722d1ea51160acf50120eb88995a5b7
Actually, there's a way to trigger this in bash < 4, I've tested it
and it works:
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -270,8 +270,8 @@ __git_index_files ()
__git_ls_files_helper "$root" "$1" |
while read -r file; do
case "$file" in
- ?*/*) echo "${file%%/*}/" ;;
- *) echo "$file " ;;
+ ?*/*) echo "${file%%/*}" ;;
+ *) echo "${file}" ;;
esac
done | sort | uniq
fi
@@ -490,6 +490,10 @@ __git_complete_index_file ()
;;
esac
+ # use a hack to enable file mode in bash < 4
+ compopt -o filenames +o nospace 2> /dev/null ||
+ compgen -f /non-existing-dir/ > /dev/null
+
__gitcomp_nl "$(__git_index_files "$1" "$pfx")" "$pfx" "$cur_" ""
}
--
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/11] completion: general cleanups
On Sat, Apr 27, 2013 at 10:40 AM, Manlio Perillo
wrote:
> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA1
>
> Il 27/04/2013 15:07, Felipe Contreras ha scritto:
>> [...]
>> 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 ()
>>
>
> I like the idea (but I have not tested it), however compopt is called
> two times, for each completion.
Why two times?
> Maybe we can test for `-o filenames` support when script is loaded,
> where currently there is a Bash version check, and set a global variable?
Yeah, that's the way bash-completion used to do it. But I wonder if we
should be worrying about this at this point, even bash-completion
dropped support for bash < 4 more than two years ago, and even debian
stable is at 4.1.
http://anonscm.debian.org/gitweb/?p=bash-completion/bash-completion.git;a=commitdiff;h=f1b3be235722d1ea51160acf50120eb88995a5b7
Cheers.
--
Felipe Contreras
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/11] completion: general cleanups
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1
Il 27/04/2013 15:07, Felipe Contreras ha scritto:
> [...]
> 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 ()
>
I like the idea (but I have not tested it), however compopt is called
two times, for each completion.
Maybe we can test for `-o filenames` support when script is loaded,
where currently there is a Bash version check, and set a global variable?
Regards Manlio
-BEGIN PGP SIGNATURE-
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iEYEARECAAYFAlF78WcACgkQscQJ24LbaUSjzgCfWq26RMqFLgGU9B8C0mb+Wogu
A5IAnjKpupGbdOZAKtYZkglYKSmbqtqK
=iTzW
-END PGP SIGNATURE-
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/11] completion: general cleanups
On Sat, Apr 27, 2013 at 7:36 AM, Felipe Contreras
wrote:
> On Sat, Apr 27, 2013 at 6:33 AM, Manlio Perillo
> 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/`, 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`.
>>
>> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/11] completion: general cleanups
On Sat, Apr 27, 2013 at 6:33 AM, Manlio Perillo 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/`, 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`. > > 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. Cheers. -- Felipe Contreras -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/11] completion: general cleanups
-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/`, 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`. As far as the "double slash" problem with the __git_index_file_list_filter_bash function, please try `git rm contrib`. 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. > [...] Regards Manlio -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.10 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iEYEARECAAYFAlF7t5gACgkQscQJ24LbaUSO5QCffllxM8RbGUP47kb7uL5J3drF hkUAn26ezKptTAC412EJZnxjh7RVcdAO =Piyz -END PGP SIGNATURE- -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
