completion: new config var to use --sort in for-each-ref #1946
Conversation
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 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 <[email protected]>
Welcome to GitGitGadgetHi @nbenitez, and welcome to GitGitGadget, the GitHub App to send patch series to the Git mailing list from GitHub Pull Requests. Please make sure that either:
You can CC potential reviewers by adding a footer to the PR description with the following syntax: NOTE: DO NOT copy/paste your CC list from a previous GGG PR's description, Also, it is a good idea to review the commit messages one last time, as the Git project expects them in a quite specific form:
It is in general a good idea to await the automated test ("Checks") in this Pull Request before contributing the patches, e.g. to avoid trivial issues such as unportable code. Contributing the patchesBefore you can contribute the patches, your GitHub username needs to be added to the list of permitted users. Any already-permitted user can do that, by adding a comment to your PR of the form Both the person who commented An alternative is the channel Once on the list of permitted usernames, you can contribute the patches to the Git mailing list by adding a PR comment If you want to see what email(s) would be sent for a After you submit, GitGitGadget will respond with another comment that contains the link to the cover letter mail in the Git mailing list archive. Please make sure to monitor the discussion in that thread and to address comments and suggestions (while the comments and suggestions will be mirrored into the PR by GitGitGadget, you will still want to reply via mail). If you do not want to subscribe to the Git mailing list just to be able to respond to a mail, you can download the mbox from the Git mailing list archive (click the curl -g --user "<EMailAddress>:<Password>" \
--url "imaps://imap.gmail.com/INBOX" -T /path/to/raw.txtTo iterate on your change, i.e. send a revised patch or patch series, you will first want to (force-)push to the same branch. You probably also want to modify your Pull Request description (or title). It is a good idea to summarize the revision by adding something like this to the cover letter (read: by editing the first comment on the PR, i.e. the PR description): To send a new iteration, just add another PR comment with the contents: Need help?New contributors who want advice are encouraged to join [email protected], where volunteers who regularly contribute to Git are willing to answer newbie questions, give advice, or otherwise provide mentoring to interested contributors. You must join in order to post or view messages, but anyone can join. You may also be able to find help in real time in the developer IRC channel, |
|
/allow |
|
User nbenitez is now allowed to use GitGitGadget. WARNING: nbenitez has no public email address set on GitHub; GitGitGadget needs an email address to Cc: you on your contribution, so that you receive any feedback on the Git mailing list. Go to https://github.com/settings/profile to make your preferred email public to let GitGitGadget know which email address to use. |
|
/submit |
|
Submitted as [email protected] To fetch this version into To fetch this version to local tag |
|
On the Git mailing list, Nelson Benítez León wrote (reply to this): > 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 "
that should be "git checkout <TAB>" (got removed by Github apparently). |
|
On the Git mailing list, Junio C Hamano wrote (reply to this): "Nelson Benitez Leon via GitGitGadget" <[email protected]>
writes:
> From: =?UTF-8?q?Nelson=20Ben=C3=ADtez=20Le=C3=B3n?= <[email protected]>
>
> 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
>
> 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.
The usual way to compose a log message of this project is to
- Give an observation on how the current system works in the
present tense (so no need to say "Currently X is Y", or
"Previously X was Y" to describe the state before your change;
just "X is Y" is enough), and discuss what you perceive as a
problem in it.
- Propose a solution (optional---often, problem description
trivially leads to an obvious solution in reader's minds).
- Give commands to somebody editing the codebase to "make it so".
There is no place to say "this commit will"; instead, give order
to the code.
in this order.
> Signed-off-by: Nelson Benítez León <[email protected]>
> ---
> I've not added tests because I'm not familiar with the test machinery,
> hopefully this is still useful.
Tests are required part of a new feature to become part of Git
codebase. Otherwise you cannot notice if somebody else later
changes the code further and accidentally breaks your feature.
You can of course ask for help (i.e. "I'm not familiar with the test
machinery so I am not writing any" is *not* acceptable. "I'm not
familiar with the test machinery; among those who may find this new
feature worthwhile to do, could somebody lend me a hand?" is very
good).
> ... and I don't want to activate 2FA
> authentication just for this ...
You could do OAuth these days, I hear? |
|
On the Git mailing list, SZEDER Gábor wrote (reply to this): On Sun, Jul 27, 2025 at 02:49:33PM +0000, Nelson Benitez Leon via GitGitGadget wrote:
> From: =?UTF-8?q?Nelson=20Ben=C3=ADtez=20Le=C3=B3n?= <[email protected]>
>
> 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?!
> 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 <[email protected]>
> ---
> 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.
> 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.
> 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.
> +}
> +
> # 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 $1
This 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.
> }
>
> # Setup the completion for git commands
>
> base-commit: e4ef0485fd78fcb05866ea78df35796b904e4a8e
> --
> gitgitgadget
> |
|
User |
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. 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:
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.
cc: SZEDER Gábor [email protected]