On 08-ene-2024 15:53:03, Britton Leo Kerin wrote:
> Signed-off-by: Britton Leo Kerin <britton.kerin@xxxxxxxxx>
> ---
> contrib/completion/git-completion.bash | 37 ++++++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 185b47d802..2b2b6c9738 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1356,6 +1356,29 @@ __git_count_arguments ()
> printf "%d" $c
> }
>
> +# Complete actual dir (not pathspec), respecting any -C options.
> +#
> +# Usage: __git_complete_refs [<option>]...
> +# --cur=<word>: The current dir to be completed. Defaults to the current word.
> +__git_complete_dir ()
> +{
> + local cur_="$cur"
> +
> + while test $# != 0; do
> + case "$1" in
> + --cur=*) cur_="${1##--cur=}" ;;
> + *) return 1 ;;
> + esac
> + shift
> + done
> +
> + # This rev-parse invocation amounts to a pwd which respects -C options
> + local context_dir=$(__git rev-parse --show-toplevel --show-prefix 2>/dev/null | paste -s -d '/' 2>/dev/null)
> + [ -d "$context_dir" ] || return 1
> +
> + COMPREPLY=$(cd "$context_dir" 2>/dev/null && compgen -d -- "$cur_")
This assignment is problematic.
First, COMPREPLY is expected to be an array. Maybe a simple change can
do the trick:
- COMPREPLY=$(cd "$context_dir" 2>/dev/null && compgen -d -- "$cur_")
+ COMPREPLY=( $(cd "$context_dir" 2>/dev/null && compgen -d -- "$cur_") )
But, what happens with directories that have SP's in its name? We're
giving wrong options:
$ mkdir one
$ mkdir "one more dir"
$ git am --directory=o<TAB><TAB>
dir more one
Setting IFS can help us:
+ local IFS=$'\n'
Now we're returning correct options:
$ mkdir one
$ mkdir "one more dir"
$ git am --directory=o<TAB><TAB>
one one more dir
Here, the user might be expecting directory names with a trailing '/',
as Bash do. Again, a simple trick:
- COMPREPLY=( $(cd "$context_dir" 2>/dev/null && compgen -d -- "$cur_") )
+ COMPREPLY=( $(cd "$context_dir" 2>/dev/null && compgen -d -S / -- "$cur_") )
Now looks better, IMO:
$ git am --directory=o<TAB><TAB>
one/ one more dir/
But, after all of this, we're going to provoke a problematic completion due
to the SP:
$ mkdir "another one"
$ git am --directory=anot<TAB><TAB>
...
$ git am --directory=another one/
We should complete to:
$ git am --directory=another\ one/
Here we need a less simple trick:
+ # 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 ||
+ true
Some commits you may find interesting:
fea16b47b6 (git-completion.bash: add support for path completion, 2013-01-11)
3ffa4df4b2 (completion: add hack to enable file mode in bash < 4, 2013-04-27)
Well, so far, so good? I'm afraid, not: What happens with other
special characters like quotes '"'?
I suggest you take a look at how we are already doing all of
considerations for other commands, like git-add.
> +}
> +
> __git_whitespacelist="nowarn warn error error-all fix"
> __git_patchformat="mbox stgit stgit-series hg mboxrd"
> __git_showcurrentpatch="diff raw"
> @@ -1374,6 +1397,10 @@ _git_am ()
> __gitcomp "$__git_whitespacelist" "" "${cur##--whitespace=}"
> return
> ;;
> + --directory=*)
> + __git_complete_dir --cur="${cur##--directory=}"
> + return
> + ;;
> --patch-format=*)
> __gitcomp "$__git_patchformat" "" "${cur##--patch-format=}"
> return
> @@ -1867,7 +1894,17 @@ __git_format_patch_extra_options="
>
> _git_format_patch ()
> {
> + case "$prev,$cur" in
> + -o,*)
> + __git_complete_dir
> + return
> + ;;
> + esac
> case "$cur" in
> + --output-directory=*)
> + __git_complete_dir --cur="${cur##--output-directory=}"
> + return
> + ;;
Adding completion for these options, and possibly opening the way for
others, is a nice improvement.
Thank you for working on this.
