completion: new config var to use --sort in for-each-ref
PATCH 1/1 completion: new config var to use --sort in for-each-ref
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.
---
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. 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(-)
contrib/completion/git-completion.bash +47 -9
@@ -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 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' +} + # 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 } # Setup the completion for git commands
base-commit: e4ef0485fd78fcb05866ea78df35796b904e4a8e
Signature · 1 line
gitgitgadget
Quoted 3 lines
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).
"Nelson Benitez Leon via GitGitGadget" <[email protected]>
Quoted 6 lines · 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.
Quoted 8 lines
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.
Quoted 6 lines
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).
Quoted 2 lines
... and I don't want to activate 2FA
authentication just for this ...
You could do OAuth these days, I hear?
Quoted 8 lines · Nelson Benitez Leon via GitGitGadget
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?!
Quoted 10 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 <[email protected]>---
completion: new config var to use --sort in for-each-ref
Quoted 8 lines
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.
Quoted 39 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/1946contrib/completion/git-completion.bash | 56 +++++++++++++++++++++----- 1 file changed, 47 insertions(+), 9 deletions(-)contrib/completion/git-completion.bash +0
-0@@ -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
contrib/completion/git-completion.bash line 91
Quoted 10 lines
contrib/completion/git-completion.bash +9
-0# (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.
Quoted 9 lines
contrib/completion/git-completion.bash +4
-0case "$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 ()
contrib/completion/git-completion.bash line 111
Quoted 10 lines
contrib/completion/git-completion.bash +10
-0+{ + 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.
Quoted 88 lines
contrib/completion/git-completion.bash +21
-7+} + # 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 ;;
contrib/completion/git-completion.bash ___git_complete () · line 4025
Quoted 8 lines
contrib/completion/git-completion.bash +3
-2@@ -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.
Quoted 8 lines
contrib/completion/git-completion.bash +0
-0} # Setup the completion for git commandsbase-commit:
e4ef0485fd78fcb05866ea78df35796b904e4a8eSignature · 1 line
gitgitgadget