* [PATCH 0/1] completion: complete dir-type option args to am, format_patch
@ 2024-01-07 21:41 Britton Leo Kerin
2024-01-09 0:53 ` [PATCH v2 " Britton Leo Kerin
[not found] ` <20240109005303.444932-1-britton.kerin@gmail.com>
0 siblings, 2 replies; 6+ messages in thread
From: Britton Leo Kerin @ 2024-01-07 21:41 UTC (permalink / raw)
To: git; +Cc: Britton Leo Kerin
I think are a few other places this could be done as well but I wanted to
start with just a couple to show the idea.
Britton Leo Kerin (1):
completion: dir-type optargs for am, format-patch
contrib/completion/git-completion.bash | 38 ++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
base-commit: e79552d19784ee7f4bbce278fe25f93fbda196fa
--
2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH v2 0/1] completion: complete dir-type option args to am, format_patch 2024-01-07 21:41 [PATCH 0/1] completion: complete dir-type option args to am, format_patch Britton Leo Kerin @ 2024-01-09 0:53 ` Britton Leo Kerin [not found] ` <20240109005303.444932-1-britton.kerin@gmail.com> 1 sibling, 0 replies; 6+ messages in thread From: Britton Leo Kerin @ 2024-01-09 0:53 UTC (permalink / raw) To: git; +Cc: Britton Leo Kerin, Junio C Hamano This revision adds missing double quotes to improve the completion situation for paths with spaces in them, and adds some comments. Britton Leo Kerin (1): completion: dir-type optargs for am, format-patch contrib/completion/git-completion.bash | 37 ++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) Range-diff against v1: 1: 2d788b0b18 ! 1: 5161304a92 completion: dir-type optargs for am, format-patch @@ contrib/completion/git-completion.bash: __git_count_arguments () printf "%d" $c } -+ +# Complete actual dir (not pathspec), respecting any -C options. +# +# Usage: __git_complete_refs [<option>]... @@ contrib/completion/git-completion.bash: __git_count_arguments () + 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 ++ [ -d "$context_dir" ] || return 1 + -+ COMPREPLY=$(cd $context_dir 2>/dev/null && compgen -d -- "$cur_") ++ 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" -- 2.43.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <20240109005303.444932-1-britton.kerin@gmail.com>]
* [PATCH v2 1/1] completion: dir-type optargs for am, format-patch [not found] ` <20240109005303.444932-1-britton.kerin@gmail.com> @ 2024-01-09 0:53 ` Britton Leo Kerin 2024-02-03 15:13 ` Rubén Justo 0 siblings, 1 reply; 6+ messages in thread From: Britton Leo Kerin @ 2024-01-09 0:53 UTC (permalink / raw) To: git; +Cc: Britton Leo Kerin 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 + ;; --thread=*) __gitcomp " deep shallow -- 2.43.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] completion: dir-type optargs for am, format-patch 2024-01-09 0:53 ` [PATCH v2 1/1] completion: dir-type optargs for am, format-patch Britton Leo Kerin @ 2024-02-03 15:13 ` Rubén Justo 2024-02-13 0:52 ` Britton Kerin 0 siblings, 1 reply; 6+ messages in thread From: Rubén Justo @ 2024-02-03 15:13 UTC (permalink / raw) To: Britton Leo Kerin, git 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_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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] completion: dir-type optargs for am, format-patch 2024-02-03 15:13 ` Rubén Justo @ 2024-02-13 0:52 ` Britton Kerin 2024-02-29 0:04 ` Rubén Justo 0 siblings, 1 reply; 6+ messages in thread From: Britton Kerin @ 2024-02-13 0:52 UTC (permalink / raw) To: Rubén Justo, git On Sat, Feb 3, 2024 at 6:13 AM Rubén Justo <rjusto@gmail.com> wrote: > > 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. Thanks for all these suggestions. Considering them and working on it some more I end up with this function: __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? If not I guess what's left is special code to make it work better with old versions of bash. I'm a little sceptical that this is worth it since bash 5 is already 5 years old and it's only completion code we're talking about but I guess it could be done. Britton ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/1] completion: dir-type optargs for am, format-patch 2024-02-13 0:52 ` Britton Kerin @ 2024-02-29 0:04 ` Rubén Justo 0 siblings, 0 replies; 6+ messages in thread From: Rubén Justo @ 2024-02-29 0:04 UTC (permalink / raw) To: Britton Kerin, git On Mon, Feb 12, 2024 at 13:52:53 -0900, Britton Kerin wrote: > __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? > If not I guess what's left is special code to make it work better with > old versions of bash. I'm a little sceptical that this is worth it > since bash 5 is already 5 years old and it's only completion code > we're talking about but I guess it could be done. I don't think you need to dig too much into old Bash versions. If it works with a recent one, it's a good start. Have you considered adding some tests to t/t9902-completion.sh? It is desirable to see some tests at least for __git_complete_dir. Perhaps it would also help you to polish the function. Sorry for the late response. I just found your message while reviewing the topics in the 'What's cooking'. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-02-29 0:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-01-07 21:41 [PATCH 0/1] completion: complete dir-type option args to am, format_patch Britton Leo Kerin
2024-01-09 0:53 ` [PATCH v2 " Britton Leo Kerin
[not found] ` <20240109005303.444932-1-britton.kerin@gmail.com>
2024-01-09 0:53 ` [PATCH v2 1/1] completion: dir-type optargs for am, format-patch Britton Leo Kerin
2024-02-03 15:13 ` Rubén Justo
2024-02-13 0:52 ` Britton Kerin
2024-02-29 0:04 ` Rubén Justo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).