Re: [PATCH 1/2] completion: add optional ignore-case when matching refs
On Sat, Nov 05, 2022 at 05:28:34PM +0000, Alison Winters via GitGitGadget wrote:
Show 25 quoted lines
> From: Alison Winters <alisonatwork@outlook.com>
>
> If GIT_COMPLETION_IGNORE_CASE=1 is set, --ignore-case will be added to
> git for-each-ref calls so that branches and tags can be matched case
> insensitively.
>
> Signed-off-by: Alison Winters <alisonatwork@outlook.com>
> ---
> contrib/completion/git-completion.bash | 41 ++++++++++++++++++++++++++
> t/t9902-completion.sh | 16 ++++++++++
> 2 files changed, 57 insertions(+)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index ba5c395d2d8..8ed96a5b8b6 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -58,6 +58,11 @@
> #
> # When set to "1" suggest all options, including options which are
> # typically hidden (e.g. '--allow-empty' for 'git commit').
> +#
> +# GIT_COMPLETION_IGNORE_CASE
> +#
> +# When set to "1", suggest refs that match case insensitively (e.g.,
> +# completing "FOO" on "git checkout f<TAB>").
I wish the commit message and this comment would be more explicit about only the matching being case insensitive, but the words listed for completion will all have the same case as the files/refs/etc. that are being completed.
I've already started writing a reply along the lines of "does this only work on case-insensitive filesystems?!" before finally realizing that's not how it works...
Show 12 quoted lines
> case "$COMP_WORDBREAKS" in
> *:*) : great ;;
> @@ -644,8 +649,15 @@ __git_complete_index_file ()
> __git_heads ()
> {
> local pfx="${1-}" cur_="${2-}" sfx="${3-}"
> + local ignore_case=""
> +
> + if test "${GIT_COMPLETION_IGNORE_CASE-}" = "1"
> + then
> + ignore_case="--ignore-case"
> + fiI find these six lines a bit too verbose for what there are doing, especially since the same six lines are added a couple of times. I think it could be shortened to just a single line with the "use alternate value" parameter expansion like this:
local ignore_case=${GIT_COMPLETION_IGNORE_CASE+--ignore-case}> __git for-each-ref --format="${pfx//\%/%%}%(refname:strip=2)$sfx" \
> + $ignore_case \In fact, we could eliminate that new $ignore_case local variable entirely by adding that parameter expansion right here.
Show 106 quoted lines
> "refs/heads/$cur_*" "refs/heads/$cur_*/**"
> }
>
> @@ -657,8 +669,15 @@ __git_heads ()
> __git_remote_heads ()
> {
> local pfx="${1-}" cur_="${2-}" sfx="${3-}"
> + local ignore_case=""
> +
> + if test "${GIT_COMPLETION_IGNORE_CASE-}" = "1"
> + then
> + ignore_case="--ignore-case"
> + fi
>
> __git for-each-ref --format="${pfx//\%/%%}%(refname:strip=2)$sfx" \
> + $ignore_case \
> "refs/remotes/$cur_*" "refs/remotes/$cur_*/**"
> }
>
> @@ -667,8 +686,15 @@ __git_remote_heads ()
> __git_tags ()
> {
> local pfx="${1-}" cur_="${2-}" sfx="${3-}"
> + local ignore_case=""
> +
> + if test "${GIT_COMPLETION_IGNORE_CASE-}" = "1"
> + then
> + ignore_case="--ignore-case"
> + fi
>
> __git for-each-ref --format="${pfx//\%/%%}%(refname:strip=2)$sfx" \
> + $ignore_case \
> "refs/tags/$cur_*" "refs/tags/$cur_*/**"
> }
>
> @@ -682,12 +708,19 @@ __git_dwim_remote_heads ()
> {
> local pfx="${1-}" cur_="${2-}" sfx="${3-}"
> local fer_pfx="${pfx//\%/%%}" # "escape" for-each-ref format specifiers
> + local ignore_case=""
> +
> + if test "${GIT_COMPLETION_IGNORE_CASE-}" = "1"
> + then
> + ignore_case="--ignore-case"
> + fi
>
> # employ the heuristic used by git checkout and git switch
> # Try to find a remote branch that cur_es the completion word
> # but only output if the branch name is unique
> __git for-each-ref --format="$fer_pfx%(refname:strip=3)$sfx" \
> --sort="refname:strip=3" \
> + $ignore_case \
> "refs/remotes/*/$cur_*" "refs/remotes/*/$cur_*/**" | \
> uniq -u
> }
> @@ -713,6 +746,7 @@ __git_refs ()
> local pfx="${3-}" cur_="${4-$cur}" sfx="${5-}"
> local match="${4-}"
> local fer_pfx="${pfx//\%/%%}" # "escape" for-each-ref format specifiers
> + local ignore_case=""
>
> __git_find_repo_path
> dir="$__git_repo_path"
> @@ -735,6 +769,11 @@ __git_refs ()
> fi
> fi
>
> + if test "${GIT_COMPLETION_IGNORE_CASE-}" = "1"
> + then
> + ignore_case="--ignore-case"
> + fi
> +
> if [ "$list_refs_from" = path ]; then
> if [[ "$cur_" == ^* ]]; then
> pfx="$pfx^"
> @@ -765,6 +804,7 @@ __git_refs ()
> ;;
> esac
> __git_dir="$dir" __git for-each-ref --format="$fer_pfx%($format)$sfx" \
> + $ignore_case \
> "${refs[@]}"
> if [ -n "$track" ]; then
> __git_dwim_remote_heads "$pfx" "$match" "$sfx"
> @@ -787,6 +827,7 @@ __git_refs ()
> $match*) echo "${pfx}HEAD$sfx" ;;
> esac
> __git for-each-ref --format="$fer_pfx%(refname:strip=3)$sfx" \
> + $ignore_case \
> "refs/remotes/$remote/$match*" \
> "refs/remotes/$remote/$match*/**"
> else
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index 43de868b800..f62a395d827 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -2255,6 +2255,22 @@ test_expect_success 'checkout completes ref names' '
> EOF
> '
>
> +test_expect_success 'checkout does not match ref names of a different case' '
> + test_completion "git checkout M" ""
> +'
> +
> +test_expect_success 'checkout matches case insensitively with GIT_COMPLETION_IGNORE_CASE' '
> + (
> + . "$GIT_BUILD_DIR/contrib/completion/git-completion.bash" &&I don't think it's necessary to source the completion script here, because the test script's main shell process has already sourced it, and its functions are visible in this test's subshell as well.
(Yeah, there are a few test cases near the end that do (re-)source the completion script, but they do so because they must invalidate the cache variables used by the completion script; this case-insensitive match feature, however, doesn't involve any of those cache variables.)
> + GIT_COMPLETION_IGNORE_CASE=1 && export GIT_COMPLETION_IGNORE_CASE &&
There is no need to export this variable, because only a handful of completion helper functions look at it, which are invoked in this same shell process (or perhaps in one of its subshells, but this variable will be visible there are all the same).
Show 14 quoted lines
> + test_completion "git checkout M" <<-\EOF
> + main Z
> + mybranch Z
> + mytag Z
> + EOF
> + )
> +'
> +
> test_expect_success 'git -C <path> checkout uses the right repo' '
> test_completion "git -C subdir -C subsubdir -C .. -C ../otherrepo checkout b" <<-\EOF
> branch-in-other Z
> --
> gitgitgadget
>
Alison Winters via GitGitGadget
