Re: [PATCH] completion: new config var to use --sort in for-each-ref
- From
SZEDER Gábor <szeder.dev@gmail.com>
- Date
- Jul 28, 2025, 17:13 UTC
- Message-ID
- <aIevs9Y9/0AlzzhP@szeder.dev>
- In-Reply-To
- <pull.1946.git.1753627773304.gitgitgadget@gmail.com>
On Sun, Jul 27, 2025 at 02:49:33PM +0000, Nelson Benitez Leon via GitGitGadget wrote:
Show 8 quoted lines
> From: =?UTF-8?q?Nelson=20Ben=C3=ADtez=20Le=C3=B3n?= <nbenitezl@gmail.com> > > Currently when completing refs, e.g. by doing "git checkout <TAB>", all > refs are shown in alphabetical order, this is an implicit ordering and > cannot be changed. > > This commit will make the sort criteria to now be explicit, mandated by > a new config var which will be used for the --sort=<val> of for-each-ref
But why would you want to use any other ordering?!
Show 18 quoted lines
> This new config var will have a default value of alphabetical order, > so Git's default behaviour remains unchanged. > > Also add '-o nosort' to 'complete' to disable its default alphabetical > ordering so our new explicit ordering prevails. > > Signed-off-by: Nelson Benítez León <nbenitezl@gmail.com> > --- > completion: new config var to use --sort in for-each-ref > > Hi, I'm submitting a patch for the Bash completion script, to be able to > change the default implicit alphabetical ordering used when returning > refs e.g. when doing "git checkout " > > I wanted the completed refs to be sorted by "recently worked on", I > achieve it by using committer date field in descending order i.e. > --sort="-committerdate" because that shows on top the branches that have > recently been worked on.
Ah, that's why :) This would be a good addition to the log message, and perhaps a sufficient justification for the proposed change.
Show 49 quoted lines
> The completion script does not allow to set a > custom sort order, so we're stuck with the default alphabetical one, so > I'm sending a patch which adds a new config var where the user can set > their desired custom sort criteria. > > I've not added tests because I'm not familiar with the test machinery, > hopefully this is still useful. > > I'd also like to ask the Git audience about their preference for > changing the default sort value in a future patch: > > 1. stay the same (alphabetical order) > 2. change it to show recently worked on branches first (like me) > > I people agree 2. is more useful then we can change it in a follow-up > patch. > > Regards, > > PD. I previously sent this to the mailing list but resulted in a bad > formatted email because I use Gmail (and I don't want to activate 2FA > authentication just for this) so I'm sending this time through > GitGitGadget and incorporating some fixes from review comments I got, > like adapting commit message to 72 chars wide. > > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1946%2Fnbenitez%2Fbash_completion_explicit_sort-v1 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1946/nbenitez/bash_completion_explicit_sort-v1 > Pull-Request: https://github.com/gitgitgadget/git/pull/1946 > > contrib/completion/git-completion.bash | 56 +++++++++++++++++++++----- > 1 file changed, 47 insertions(+), 9 deletions(-) > > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash > index e3d88b06721..59964a8056e 100644 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -80,12 +80,37 @@ > # When set, uses for-each-ref '--ignore-case' to find refs that match > # case insensitively, even on systems with case sensitive file systems > # (e.g., completing tag name "FOO" on "git checkout f<TAB>"). > +# > +# GIT_COMPLETION_REFS_SORT_BY_FIELDNAME > +# > +# Fieldname string to use for --sort option of for-each-ref. If empty or > +# not defined it defaults to "refname" which is the same default git uses > +# when no --sort option is provided. Some example values: > +# '-committerdate' to descending sort by committer date > +# '-version:refname' to descending sort by refname interpreted as version > +# More info and examples: https://git-scm.com/docs/git-for-each-ref#_field_names
This approach allows only one sort key to be specified, although 'git for-each-ref' supports more by accepting multiple --sort options.
Show 19 quoted lines
> case "$COMP_WORDBREAKS" in
> *:*) : great ;;
> *) COMP_WORDBREAKS="$COMP_WORDBREAKS:"
> esac
>
> +# Reads and validates GIT_COMPLETION_REFS_SORT_BY_FIELDNAME configuration var,
> +# returning the content of it when it's valid, or if not valid or is empty or
> +# not defined, then it returns the documented default i.e. 'refname'.
> +__git_get_sort_by_fieldname ()
> +{
> + if [ -n "${GIT_COMPLETION_REFS_SORT_BY_FIELDNAME-}" ]; then
> + # Validate by using a regex pattern which only allows a set
> + # of characters that may appear in a --sort expression
> + if [[ "$GIT_COMPLETION_REFS_SORT_BY_FIELDNAME" =~ ^[a-zA-Z0-9%:=*(),_\ -]+$ ]]; then
> + echo "$GIT_COMPLETION_REFS_SORT_BY_FIELDNAME"
> + return
> + fi
> + fi
> + echo 'refname'Printing the sort key to the function's stdout requires that it's called in a command substitution. Forking a subshell for a command substitution is very expensive on Windows, therefore we should try to avoid that in new helper functions, if possible.
Please consider returning the sort key in a particular variable that is specified as local in the function's callers. See __git_decode() for an example.
Show 96 quoted lines
> +}
> +
> # Discovers the path to the git repository taking any '--git-dir=<path>' and
> # '-C <path>' options into account and stores it in the $__git_repo_path
> # variable.
> @@ -751,7 +776,9 @@ __git_heads ()
> {
> local pfx="${1-}" cur_="${2-}" sfx="${3-}"
>
> - __git for-each-ref --format="${pfx//\%/%%}%(refname:strip=2)$sfx" \
> + local sortby=$(__git_get_sort_by_fieldname)
> +
> + __git for-each-ref --sort="$sortby" --format="${pfx//\%/%%}%(refname:strip=2)$sfx" \
> ${GIT_COMPLETION_IGNORE_CASE+--ignore-case} \
> "refs/heads/$cur_*" "refs/heads/$cur_*/**"
> }
> @@ -765,7 +792,9 @@ __git_remote_heads ()
> {
> local pfx="${1-}" cur_="${2-}" sfx="${3-}"
>
> - __git for-each-ref --format="${pfx//\%/%%}%(refname:strip=2)$sfx" \
> + local sortby=$(__git_get_sort_by_fieldname)
> +
> + __git for-each-ref --sort="$sortby" --format="${pfx//\%/%%}%(refname:strip=2)$sfx" \
> ${GIT_COMPLETION_IGNORE_CASE+--ignore-case} \
> "refs/remotes/$cur_*" "refs/remotes/$cur_*/**"
> }
> @@ -776,7 +805,9 @@ __git_tags ()
> {
> local pfx="${1-}" cur_="${2-}" sfx="${3-}"
>
> - __git for-each-ref --format="${pfx//\%/%%}%(refname:strip=2)$sfx" \
> + local sortby=$(__git_get_sort_by_fieldname)
> +
> + __git for-each-ref --sort="$sortby" --format="${pfx//\%/%%}%(refname:strip=2)$sfx" \
> ${GIT_COMPLETION_IGNORE_CASE+--ignore-case} \
> "refs/tags/$cur_*" "refs/tags/$cur_*/**"
> }
> @@ -818,7 +849,9 @@ __git_dwim_remote_heads ()
> }
> }
> '
> - __git for-each-ref --format='%(refname)' refs/remotes/ |
> + local sortby=$(__git_get_sort_by_fieldname)
> +
> + __git for-each-ref --sort="$sortby" --format='%(refname)' refs/remotes/ |
> PFX="$pfx" SFX="$sfx" CUR_="$cur_" \
> IGNORE_CASE=${GIT_COMPLETION_IGNORE_CASE+1} \
> REMOTES="$(__git_remotes | sort -r)" awk "$awk_script" |
> @@ -847,6 +880,7 @@ __git_refs ()
> local match="${4-}"
> local umatch="${4-}"
> local fer_pfx="${pfx//\%/%%}" # "escape" for-each-ref format specifiers
> + local sortby=$(__git_get_sort_by_fieldname)
>
> __git_find_repo_path
> dir="$__git_repo_path"
> @@ -905,7 +939,8 @@ __git_refs ()
> "refs/remotes/$match*" "refs/remotes/$match*/**")
> ;;
> esac
> - __git_dir="$dir" __git for-each-ref --format="$fer_pfx%($format)$sfx" \
> + __git_dir="$dir" __git for-each-ref --sort="$sortby" \
> + --format="$fer_pfx%($format)$sfx" \
> ${GIT_COMPLETION_IGNORE_CASE+--ignore-case} \
> "${refs[@]}"
> if [ -n "$track" ]; then
> @@ -929,7 +964,8 @@ __git_refs ()
> $match*|$umatch*) echo "${pfx}HEAD$sfx" ;;
> esac
> local strip="$(__git_count_path_components "refs/remotes/$remote")"
> - __git for-each-ref --format="$fer_pfx%(refname:strip=$strip)$sfx" \
> + __git for-each-ref --sort="$sortby" \
> + --format="$fer_pfx%(refname:strip=$strip)$sfx" \
> ${GIT_COMPLETION_IGNORE_CASE+--ignore-case} \
> "refs/remotes/$remote/$match*" \
> "refs/remotes/$remote/$match*/**"
> @@ -2861,7 +2897,8 @@ __git_complete_config_variable_value ()
> remote.*.push)
> local remote="${varname#remote.}"
> remote="${remote%.push}"
> - __gitcomp_nl "$(__git for-each-ref \
> + local sortby=$(__git_get_sort_by_fieldname)
> + __gitcomp_nl "$(__git for-each-ref --sort="$sortby" \
> --format='%(refname):%(refname)' refs/heads)" "" "$cur_"
> return
> ;;
> @@ -3983,8 +4020,9 @@ ___git_complete ()
> {
> local wrapper="__git_wrap${2}"
> eval "$wrapper () { __git_func_wrap $2 ; }"
> - complete -o bashdefault -o default -o nospace -F $wrapper $1 2>/dev/null \
> - || complete -o default -o nospace -F $wrapper $1
> + complete -o bashdefault -o default -o nospace -o nosort \
> + -F $wrapper $1 2>/dev/null \
> + || complete -o default -o nospace -o nosort -F $wrapper $1This is problematic, because it turns off sorting for all completion invocations, but in many cases we do need Bash to do the sorting for us:
- Subcommands and --options still hard-coded in the completion
script are usually listed in arbitrary order.
- Subcommands and --options listed programmatically by the
parse-options machinery are listed in the order they are specified
in the C source files (which tends to be the order that makes most
sense for the help output).
- Some completion functions list possible completion words from
multiple sources.I'm afraid that any change that leaves these cases unsorted is unacceptable.
Show 8 quoted lines
> } > > # Setup the completion for git commands > > base-commit: e4ef0485fd78fcb05866ea78df35796b904e4a8e > -- > gitgitgadget >