patchcompletion: complete dir-type option args to am, format_patch

> Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
> ---
>  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_")

> +}
> +
>  __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
> +		;;

>
> On 08-ene-2024 15:53:03, Britton Leo Kerin wrote:
> > Signed-off-by: Britton Leo Kerin <britton.kerin@gmail.com>
> > ---
> >  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_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
> 
>         compopt -o noquote
> 
>         local IFS=$'\n'
>         local unescaped_candidates=($(cd "$context_dir" 2>/dev/null &&
> compgen -d -S / -- "$cur_"))
>         for ii in "${!unescaped_candidates[@]}"; do
>                 COMPREPLY[$ii]=$(printf "%q" "${unescaped_candidates[$ii]}")
>         done
> }
> 
> This one works for all weird characters that I've tried in bash 5.2 at
> least, and in frameworks that do their own escaping also (e.g.
> ble.sh).  Since your advice so far was so good I thought I'd ask if
> there is anything obvious to you that is still wrong here?