Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved completions [for #145] #128

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
Open
Changes from 8 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
511a4fa
style(completions): wrap into a function
Farid-NL May 20, 2024
1e9e3fd
style(completions): use double quotes only when necessary
Farid-NL May 20, 2024
dadae45
style(completions): separate options from commands
Farid-NL May 20, 2024
cd0e5cf
feat(completions): list abbreviations for erase and rename commands
Farid-NL May 20, 2024
2605e60
fix(completions): show correctly double quotes
Farid-NL May 20, 2024
115ce50
feat(completions): group related options so it won't show them if an …
Farid-NL May 20, 2024
5d23c24
fix(completions): double <tab> was required to invoke completions
Farid-NL May 20, 2024
110adac
feat(completions): list abbreviations given scope and type options if…
Farid-NL May 20, 2024
090ebc3
style(completions): use double quotes throughout the file
Farid-NL May 21, 2024
c6c4b01
style(completions): alphabetize options
Farid-NL May 21, 2024
77cd21a
style(completions): fix indentation
Farid-NL May 21, 2024
1164090
revert(completions): use _values intead of _describe for main cmds
Farid-NL May 21, 2024
f7b49a1
refactor(completions): use a single function for completing abbreviat…
Farid-NL May 21, 2024
2643344
fix(completions): escape colons
Farid-NL Jun 10, 2024
408204c
fix(completions): remove only the initial and final double quotation …
Farid-NL Jun 11, 2024
7eab322
fix(completions): survives user sourcing zshrc
olets Aug 1, 2024
d6b89de
refactor(completions): __abbr_abbreviations dupes _abbr:util_list
olets Aug 1, 2024
ce877cc
refactor(_abbr:util_list, completions' __abbr_abbreviations): tighten
olets Aug 1, 2024
67459eb
style: one local var per local call
olets Aug 1, 2024
f484495
refactor(completions): rename fns, vars to match docs + add docs comment
olets Aug 1, 2024
afacb69
fix(completions): escape colons in _describe
olets Aug 1, 2024
c6322a2
fix(completions): scope and type are independent of each other
olets Aug 1, 2024
7fe7d1d
fix(completions): more reliable handling of colons in abbreviations
olets Aug 1, 2024
a2624d6
docs(roadmap): check off command completion
olets Oct 26, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
369 changes: 184 additions & 185 deletions completions/_abbr
Original file line number Diff line number Diff line change
Expand Up @@ -10,197 +10,196 @@
#
# ------------------------------------------------------------------------------

_abbr (){
local state line

local state line
local -i ret

local -i ret
ret=1

ret=1

_arguments -C \
_arguments -C \
'--help[Show the manpage.]' \
'(-v --version)'{-v,--version}'[Show the current version.]' \
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

revert pls. I hear your UX expectation, as well as the implicit "these aren't commands so they shouldn't be in cmds". But in the special cases of help and version I prefer to eagerly surface their existence, instead of only after -

Copy link
Author

@Farid-NL Farid-NL May 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha, I will revert it.

Since _values has a similar syntax to _arguments, we could group the related commands the same way, but this may be unnecessary?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 1164090

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since _values has a similar syntax to _arguments, we could group the related commands the same way, but this may be unnecessary

Say more?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, an example would be:

{a,add}"[Add a new abbreviation.]" \
{c,clear-session}"[Erase all session abbreviations.]" \
# ...
{R,rename}"[Rename an abbreviation.]" \
{version,-v,--version}"[Show the current version.]"

In this case I don't think it would ne needed to use the parentheses syntax, i.e.

"(R rename)"{R,rename}"[Rename an abbreviation.]"

for excluding related commands/options, since in this context only one can be used and suggested.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can click resolve on this. If you agree @Farid-NL, go ahead and do it

'1: :->cmds' \
'*:: :->args' && ret=0

case $state in
cmds)
_values "abbr command" \
"a[Add a new abbreviation.]" \
"add[Add a new abbreviation.]" \
"c[Erase all session abbreviations.]" \
"clear-session[Erase all session abbreviations.]" \
"e[Erase an abbreviation.]" \
"erase[Erase an abbreviation.]" \
"x[Output the ABBREVIATION's EXPANSION.]" \
"expand[Output the ABBREVIATION's EXPANSION.]" \
"export-aliases[Export abbreviations as alias commands.]" \
"g[Add a regular abbreviation, the expansion of which is prefixed with git; and add a global abbreviation, the abbreviation and expansion of which are prefixed with git.]" \
"git[Add a regular abbreviation, the expansion of which is prefixed with git; and add a global abbreviation, the abbreviation and expansion of which are prefixed with git.]" \
"help[Show the manpage.]" \
"--help[Show the manpage.]" \
"import-aliases[Add regular abbreviations for every regular alias in the session, and global abbreviations for every global alias in the session.]" \
"import-fish[Import fish abbr-syntax abbreviations.]" \
"import-git-aliases[Add regular abbreviations for every Git alias in the current session. The EXPANSION is prefixed with git\[Space\].]" \
"list[List the abbreviations with their expansions.]" \
"l[List the abbreviations only.]" \
"list-abbreviations[List the abbreviations only.]" \
"list-commands[List as commands suitable for export.]" \
"profile[Log profile information for debugging.]" \
"R[Rename an abbreviation.]" \
"rename[Rename an abbreviation.]" \
"version[Show the current version.]" \
"-v[Show the current version.]" \
"--version[Show the current version.]"
ret=0
;;
args)
case $line[1] in
a|\
add)
# [<SCOPE>] [<TYPE>] [--dry-run] [(--quiet | --quieter)] [(-f | --force)] ABBREVIATION=EXPANSION
_arguments \
"(--dry-run)--dry-run[see what would result, without making any actual changes]" \
"(-f)-f[ignore warnings]" \
"(--force)--force[ignore warnings]" \
"(-g)-g[expand everywhere]" \
"(--global)--global[expand everywhere]" \
"(-q)-q[silence success output]" \
"(--qq)--qq[silence success output and warnings]" \
"(--quiet)--quiet[silence success output]" \
"(--quieter)--quieter[silence success output and warnings]" \
"(-r)-r[expand at the start of the line]" \
"(--regular)--regular[expand at the start of the line]" \
"(-S)-S[available in this session]" \
"(--session)--session[available in this session]" \
"(-U)-U[available in all sessions]" \
"(--user)--user[available in all sessions]"
ret=0
;;
e|\
erase)
# [<SCOPE>] [<TYPE>] [--dry-run] [--quiet] ABBREVIATION
_arguments \
"(--dry-run)--dry-run[see what would result, without making any actual changes]" \
"(-g)-g[expand everywhere]" \
"(--global)--global[expand everywhere]" \
"(-q)-q[silence success output]" \
"(--quiet)--quiet[silence success output]" \
"(-r)-r[expand at the start of the line]" \
"(--regular)--regular[expand at the start of the line]" \
"(-S)-S[available in this session]" \
"(--session)--session[available in this session]" \
"(-U)-U[available in all sessions]" \
"(--user)--user[available in all sessions]"
ret=0
;;
export-aliases|\
list|\
l|\
list-abbreviations|\
L|\
list-commands)
# [<SCOPE>] [<TYPE>]
_arguments \
"(--global)--global[expand everywhere]" \
"(-g)-g[expand everywhere]" \
"(-r)-r[expand at the start of the line]" \
"(--regular)--regular[expand at the start of the line]" \
"(-S)-S[available in this session]" \
"(--session)--session[available in all sessions]" \
"(--user)--user[available in all sessions]" \
"(-U)-U[available in all sessions]"
case $state in
cmds)
local commands; commands=(
olets marked this conversation as resolved.
Show resolved Hide resolved
'a:Add a new abbreviation.'
'add:Add a new abbreviation.'
'c:Erase all session abbreviations.'
'clear-session:Erase all session abbreviations.'
'e:Erase an abbreviation.'
'erase:Erase an abbreviation.'
"x:Output the ABBREVIATION's EXPANSION."
"expand:Output the ABBREVIATION's EXPANSION."
'export-aliases:Export abbreviations as alias commands.'
'g:Add a regular abbreviation, the expansion of which is prefixed with git; and add a global abbreviation, the abbreviation and expansion of which are prefixed with git.'
'git:Add a regular abbreviation, the expansion of which is prefixed with git; and add a global abbreviation, the abbreviation and expansion of which are prefixed with git.'
'help:Show the manpage.'
'import-aliases:Add regular abbreviations for every regular alias in the session, and global abbreviations for every global alias in the session.'
'import-fish:Import fish abbr-syntax abbreviations.'
'import-git-aliases:Add regular abbreviations for every Git alias in the current session. The EXPANSION is prefixed with git[Space].'
'list:List the abbreviations with their expansions.'
'l:List the abbreviations only.'
'list-abbreviations:List the abbreviations only.'
'list-commands:List as commands suitable for export.'
'profile:Log profile information for debugging.'
'R:Rename an abbreviation.'
'rename:Rename an abbreviation.'
'version:Show the current version.'
)
_describe 'abbr commands' commands
olets marked this conversation as resolved.
Show resolved Hide resolved
ret=0
;;
args)
case $line[1] in
a|\
add)
# [<SCOPE>] [<TYPE>] [--dry-run] [(--quiet | --quieter)] [(-f | --force)] ABBREVIATION=EXPANSION
_arguments \
'(-S --session -U --user)'{-S,--session}'[available in this session]' \
'(-U --user -S --session)'{-U,--user}'[available in all sessions]' \
'(-r --regular -g --global)'{-r,--regular}'[expand at the start of the line]' \
'(-g --global -r --regular)'{-g,--global}'[expand everywhere]' \
'(--dry-run)--dry-run[see what would result, without making any actual changes]' \
'(-q --quiet -qq --quieter)'{-q,--quiet}'[silence success output]' \
'(-qq --quieter -q --quiet)'{-qq,--quieter}'[silence success output and warnings]' \
'(-f --force)'{-f,--force}'[ignore warnings]'
olets marked this conversation as resolved.
Show resolved Hide resolved
ret=0
;;
e|\
erase)
# [<SCOPE>] [<TYPE>] [--dry-run] [--quiet] ABBREVIATION
_arguments \
'(-S --session -U --user)'{-S,--session}'[available in this session]' \
'(-U --user -S --session)'{-U,--user}'[available in all sessions]' \
'(-r --regular -g --global)'{-r,--regular}'[expand at the start of the line]' \
'(-g --global -r --regular)'{-g,--global}'[expand everywhere]' \
'(--dry-run)--dry-run[see what would result, without making any actual changes]' \
'(-q --quiet)'{-q,--quiet}'[silence success output]' \
'1: :__abbr_abbreviations'
ret=0
;;
export-aliases|\
list|\
l|\
list-abbreviations|\
L|\
list-commands)
# [<SCOPE>] [<TYPE>]
_arguments \
'(-S --session -U --user)'{-S,--session}'[available in this session]' \
'(-U --user -S --session)'{-U,--user}'[available in all sessions]' \
'(-r --regular -g --global)'{-r,--regular}'[expand at the start of the line]' \
'(-g --global -r --regular)'{-g,--global}'[expand everywhere]'
ret=0
;;
Farid-NL marked this conversation as resolved.
Show resolved Hide resolved
git)
# [<SCOPE>] [--dry-run] [(--quiet | --quieter)] [(-f | --force)] ABBREVIATION=EXPANSION
_arguments \
'(-S --session -U --user)'{-S,--session}'[available in this session]' \
'(-U --user -S --session)'{-U,--user}'[available in all sessions]' \
'(--dry-run)--dry-run[see what would result, without making any actual changes]' \
'(-q --quiet -qq --quieter)'{-q,--quiet}'[silence success output]' \
'(-qq --quieter -q --quiet)'{-qq,--quieter}'[silence success output and warnings]' \
'(-f --force)'{-f,--force}'[ignore warnings]'
ret=0
;;
git)
# [<SCOPE>] [--dry-run] [(--quiet | --quieter)] [(-f | --force)] ABBREVIATION=EXPANSION
_arguments \
"(--dry-run)--dry-run[see what would result, without making any actual changes]" \
"(-f)-f[ignore warnings]" \
"(--force)--force[ignore warnings]" \
"(-q)-q[silence success output]" \
"(--qq)--qq[silence success output and warnings]" \
"(--quiet)--quiet[silence success output]" \
"(--quieter)--quieter[silence success output and warnings]" \
"(-S)-S[available in this session]" \
"(--session)--session[available in all sessions]" \
"(-U)-U[available in all sessions]" \
"(--user)--user[available in all sessions]"
ret=0
;;
import-aliases)
# [<TYPE>] [--dry-run] [(--quiet | --quieter)] [(-f | --force)]
_arguments \
"(--dry-run)--dry-run[see what would result, without making any actual changes]" \
"(-f)-f[ignore warnings]" \
"(--force)--force[ignore warnings]" \
"(-g)-g[expand everywhere]" \
"(--global)--global[expand everywhere]" \
"(-q)-q[silence success output]" \
"(--qq)--qq[silence success output and warnings]" \
"(--quiet)--quiet[silence success output]" \
"(--quieter)--quieter[silence success output and warnings]" \
"(-r)-r[expand at the start of the line]" \
"(--regular)--regular[expand at the start of the line]"
import-aliases)
# [<TYPE>] [--dry-run] [(--quiet | --quieter)] [(-f | --force)]
_arguments \
'(-r --regular -g --global)'{-r,--regular}'[expand at the start of the line]' \
'(-g --global -r --regular)'{-g,--global}'[expand everywhere]' \
'(--dry-run)--dry-run[see what would result, without making any actual changes]' \
'(-q --quiet -qq --quieter)'{-q,--quiet}'[silence success output]' \
'(-qq --quieter -q --quiet)'{-qq,--quieter}'[silence success output and warnings]' \
'(-f --force)'{-f,--force}'[ignore warnings]'
ret=0
;;
import-fish)
# [<SCOPE>] [--dry-run] [(--quiet | --quieter)] [(-f | --force)] FILE
_arguments \
'(-S --session -U --user)'{-S,--session}'[available in this session]' \
'(-U --user -S --session)'{-U,--user}'[available in all sessions]' \
'(--dry-run)--dry-run[see what would result, without making any actual changes]' \
'(-q --quiet -qq --quieter)'{-q,--quiet}'[silence success output]' \
'(-qq --quieter -q --quiet)'{-qq,--quieter}'[silence success output and warnings]' \
'(-f --force)'{-f,--force}'[ignore warnings]'
ret=0
;;
import-fish)
# [<SCOPE>] [--dry-run] [(--quiet | --quieter)] [(-f | --force)] FILE
_arguments \
"(--dry-run)--dry-run[see what would result, without making any actual changes]" \
"(-f)-f[ignore warnings]" \
"(--force)--force[ignore warnings]" \
"(-q)-q[silence success output]" \
"(--qq)--qq[silence success output and warnings]" \
"(--quiet)--quiet[silence success output]" \
"(--quieter)--quieter[silence success output and warnings]" \
"(-S)-S[available in this session]" \
"(--session)--session[available in this session]" \
"(-U)-U[available in all sessions]" \
"(--user)--user[available in all sessions]"
ret=0
;;
import-git-aliases)
# [<SCOPE>] [<TYPE>] [--dry-run] [(--quiet | --quieter)] [(-f | --force)] [--file <config-file>] [--prefix <ABBREVIATION prefix>]
_arguments \
"(--dry-run)--dry-run[see what would result, without making any actual changes]" \
"(--file)--file[path to a Git config file]:file:_files -/" \
"(-f)-f[ignore warnings]" \
"(--force)--force[ignore warnings]" \
"(-g)-g[expand everywhere]" \
"(--global)--global[expand everywhere]" \
"(-q)-q[silence success output]" \
"(--prefix)--prefix[prefix added to the ABBREVIATIONs]" \
"(--qq)--qq[silence success output and warnings]" \
"(--quiet)--quiet[silence success output]" \
"(--quieter)--quieter[silence success output and warnings]" \
"(-r)-r[expand at the start of the line]" \
"(--regular)--regular[expand at the start of the line]" \
"(-S)-S[available in this session]" \
"(--session)--session[available in all sessions]" \
"(-U)-U[available in all sessions]" \
"(--user)--user[available in all sessions]"
ret=0
;;
R|\
rename)
# [<SCOPE>] [<TYPE>] [--dry-run] [(--quiet | --quieter)] [(-f | --force)] OLD NEW
_arguments \
"(--dry-run)--dry-run[see what would result, without making any actual changes]" \
"(-f)-f[ignore warnings]" \
"(--force)--force[ignore warnings]" \
"(-g)-g[expand everywhere]" \
"(--global)--global[expand everywhere]" \
"(-q)-q[silence success output]" \
"(--qq)--qq[silence success output and warnings]" \
"(--quiet)--quiet[silence success output]" \
"(--quieter)--quieter[silence success output and warnings]" \
"(-r)-r[expand at the start of the line]" \
"(--regular)--regular[expand at the start of the line]" \
"(-S)-S[available in this session]" \
"(--session)--session[available in this session]" \
"(-U)-U[available in all sessions]" \
"(--user)--user[available in all sessions]"
ret=0
;;
esac
;;
esac
import-git-aliases)
# [<SCOPE>] [<TYPE>] [--dry-run] [(--quiet | --quieter)] [(-f | --force)] [--file <config-file>] [--prefix <ABBREVIATION prefix>]
_arguments \
'(-S --session -U --user)'{-S,--session}'[available in this session]' \
'(-U --user -S --session)'{-U,--user}'[available in all sessions]' \
'(-r --regular -g --global)'{-r,--regular}'[expand at the start of the line]' \
'(-g --global -r --regular)'{-g,--global}'[expand everywhere]' \
'(--dry-run)--dry-run[see what would result, without making any actual changes]' \
'(-q --quiet -qq --quieter)'{-q,--quiet}'[silence success output]' \
'(-qq --quieter -q --quiet)'{-qq,--quieter}'[silence success output and warnings]' \
'(-f --force)'{-f,--force}'[ignore warnings]' \
'(--file)--file[path to a Git config file]:filename:_files -/' \
'(--prefix)--prefix[prefix added to the ABBREVIATIONs]'
ret=0
;;
R|\
rename)
# [<SCOPE>] [<TYPE>] [--dry-run] [(--quiet | --quieter)] [(-f | --force)] OLD NEW
_arguments \
'(-S --session -U --user)'{-S,--session}'[available in this session]' \
'(-U --user -S --session)'{-U,--user}'[available in all sessions]' \
'(-r --regular -g --global)'{-r,--regular}'[expand at the start of the line]' \
'(-g --global -r --regular)'{-g,--global}'[expand everywhere]' \
'(--dry-run)--dry-run[see what would result, without making any actual changes]' \
'(-q --quiet -qq --quieter)'{-q,--quiet}'[silence success output]' \
'(-qq --quieter -q --quiet)'{-qq,--quieter}'[silence success output and warnings]' \
'(-f --force)'{-f,--force}'[ignore warnings]' \
'1: :__abbr_abbreviations'
ret=0
;;
esac
;;
esac

return ret
}

(( $+functions[__abbr_extract_abbreviations] )) ||
__abbr_extract_abbreviations() {
local -a tmp_abbr tmp_expd
local scope type

if (( $words[(Ie)-U] || $words[(Ie)--user] )); then
scope='-U'
elif (( $words[(Ie)-S] || $words[(I)--session] )); then
scope='-S'
elif (( $words[(Ie)-r] || $words[(Ie)--regular] )); then
type='-r'
elif (( $words[(Ie)-g] || $words[(Ie)--global] )); then
type='-g'
fi

tmp_abbr=(${${(f)"$(_call_program abbreviations abbr list-abbreviations $scope $type 2>/dev/null)"}//\"/})
tmp_expd=(${${${${(f)"$(_call_program abbreviations abbr list $scope $type 2>/dev/null)"}#*=}#\"}%\"})

if (( ${#ABBR_REGULAR_USER_ABBREVIATIONS} )); then
local len=${#tmp_abbr}
for (( i = 1; i <= len; i++ )); do
abbreviations+=(${tmp_abbr[$i]}:${tmp_expd[$i]})
done
else
abbreviations=()
fi
}
olets marked this conversation as resolved.
Show resolved Hide resolved

(( $+functions[__abbr_abbreviations] )) ||
__abbr_abbreviations() {
local -a abbreviations
__abbr_extract_abbreviations
olets marked this conversation as resolved.
Show resolved Hide resolved

_describe -t abbreviations 'abbr abbreviations' abbreviations
}

return ret
_abbr
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there precedent for wrapping this in a function? (it's been a while since I studied others' completion files) Is it advantageous?

Any risk of confusing the shell by giving it the same name as the file? at one point I had a function in zsh-abbr.zsh called _abbr, and that tripped up the completion system.

Copy link
Author

@Farid-NL Farid-NL May 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there precedent for wrapping this in a function? (it's been a while since I studied others' completion files) Is it advantageous?

There's a lot of discrepancies for the structure of completions as far as I can see. So my reasoning was that return is valid inside functions.

I've read mainly these completions:

In some of those files they don't even return anything or wrap the logic in a main function _<command_name>(), some of them do. It seems that when there's some complex logic behind the completions they tend to use a wrapper function.


Any risk of confusing the shell by giving it the same name as the file? at one point I had a function in zsh-abbr.zsh called _abbr, and that tripped up the completion system.

None as far as I know if it's only used in the completion file with a wrapper function, this is used by git and docker completions (from the examples above)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I unwrapped the main logic and place the helper functions above it. It seems to work fine. If you want we can do this.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Undecided.

I feel like zsh completions files are already so opaque, simplify as much as possible so give the dev community examples that aren't impossible to understand. And it's been a while since I looked, but I'm pretty sure I've seen a bunch of others that don't wrap. On the other hand, my convention is to wrap shell script file contents in a function!

Let's come back to it after everything else is finalized.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can click resolve on this. If you agree @Farid-NL, go ahead and do it