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

completion: add zsh #653

Merged
merged 11 commits into from
Aug 13, 2022
Merged

completion: add zsh #653

merged 11 commits into from
Aug 13, 2022

Conversation

ee7
Copy link
Member

@ee7 ee7 commented Aug 7, 2022

Add a zsh completion script and the ability to run

configlet completion --shell zsh

to write that script to stdout.

This commit also bumps the version of the shellcheck action, which is
necessary to prevent the action from running shellcheck on the new zsh
script. Shellcheck does not support zsh, so CI would indicate an error
otherwise.

The bump must be to the latest commit - we already used the latest
release (1.1.0). An upstream commit that claims to exclude zsh
scripts seems to be insufficient - we need to use the ignore_names
input that does not exist in the latest release.

Closes: #633


It seems like every tutorial for this is pretty bad.

Some links for the reviewer:


To test this PR, copy configlet.zsh to some directory, naming it _configlet:

mkdir ~/configlet-comp
cp completions/configlet.zsh ~/configlet-comp/_configlet

(Don't put it under /tmp, or you might get an error message about an insecure completions folder).

Then add that directory to your fpath. In .zshrc:

fpath=(~/configlet-comp $fpath)

And make sure that completions are also enabled in that file:

autoload -Uz compinit promptinit
compinit

Then start a new zsh shell, and configlet [Tab] should work.

The underlying issue is that the default fpath includes just /usr/local/share/zsh/site-functions and directories in /usr/share/zsh. We can't just write, as we can for bash:

configlet completion -s bash | source

or add the completions to some standard user completions directory, like for fish:

configlet completion -s fish > ~/.config/fish/completions/configlet.fish

So I think we have to provide instructions to make the user save the output of configlet completion --shell zsh to either:

  1. /usr/local/share/zsh/site-functions/_configlet
  2. /usr/share/zsh/site-functions/_configlet
  3. ~/foo/_configlet, explaining how to add ~/foo to their $fpath

where we probably should do the last, because:

  • e.g. generally only packages installed with the package manager should put stuff in /usr/share/zsh/site-functions.
  • the first two require root permissions.

ee7 added 10 commits August 7, 2022 15:57
We were already using the latest released version of this action, but it
runs shellcheck on files ending in `.zsh`. Shellcheck does not support
zsh, so this produces an error.

The latest release of the action only supports ignoring directories, not
files. And we shouldn't ignore the completions directory, because we
want to run shellcheck on the `configlet.bash` completion script.

For now, it looks like the best solution is to bump to the latest
ref (2021-11-14). This includes an upstream commit [1] that is supposed
to exclude `.zsh` files, but it doesn't seem to work. We need to use the
`ignore_names` input that does not exist in the latest release.

Running shellcheck on `configlet.zsh` produces these errors:

    In completions/configlet.zsh line 1:
    #compdef configlet
    ^-- SC2148 (error): Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.

    In completions/configlet.zsh line 25:
      words=($line[1] "${words[@]}")
             ^-- SC1087 (error): Use braces when expanding arrays, e.g. ${array[idx]} (or ${var}[.. to quiet).
             ^---^ SC2206 (warning): Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a.

    In completions/configlet.zsh line 27:
      curcontext="${curcontext%:*:*}:configlet-command-$line[1]:"
                                                       ^-- SC1087 (error): Use braces when expanding arrays, e.g. ${array[idx]} (or ${var}[.. to quiet).

    In completions/configlet.zsh line 28:
      case $line[1] in
           ^-- SC1087 (error): Use braces when expanding arrays, e.g. ${array[idx]} (or ${var}[.. to quiet).

    In completions/configlet.zsh line 95:
    (( $+functions[_configlet_commands] )) ||
         ^----------------------------^ SC2154 (warning): functions is referenced but not assigned.
                   ^-----------------^ SC2154 (warning): _configlet_commands is referenced but not assigned.

    In completions/configlet.zsh line 98:
      commands=(
      ^------^ SC2034 (warning): commands appears unused. Verify use (or export if used externally).

    For more information:
      https://www.shellcheck.net/wiki/SC1087 -- Use braces when expanding arrays,...
      https://www.shellcheck.net/wiki/SC2148 -- Tips depend on target shell and y...
      https://www.shellcheck.net/wiki/SC2034 -- commands appears unused. Verify u...

[1] ludeeus/action-shellcheck@ceeca77f6538
@ee7 ee7 marked this pull request as ready for review August 8, 2022 15:05
Comment on lines 50 to 64
_configlet_complete_any_exercise_slug() {
local -a cmd slugs slug_paths
slug_paths=(./exercises/concept/*(/))
slugs=( ${${slug_paths#./exercises/concept/}%-*-*} )
slug_paths=(./exercises/practice/*(/))
slugs+=( ${${slug_paths#./exercises/practice/}%-*-*} )
compadd "$@" -a slugs
}

_configlet_complete_practice_exercise_slug() {
local -a cmd slugs slug_paths
slug_paths=(./exercises/practice/*(/))
slugs=( ${${slug_paths#./exercises/practice/}%-*-*} )
compadd "$@" -a slugs
}
Copy link
Member Author

@ee7 ee7 Aug 8, 2022

Choose a reason for hiding this comment

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

Yes, this could use a refactor.

Note that this can suggest invalid slugs, because it:

Let's handle that later - we should fix it for the bash and fish completion scripts too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Refactored somewhat in 833522b

(uuid)
_arguments "${_arguments_options[@]}" \
"$_configlet_global_opts[@]" \
'(-n --num)'{-n+,--num=}'[How many UUIDs]:' \
Copy link
Member Author

Choose a reason for hiding this comment

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

-optname+
The first argument may appear immediately after optname in the same word, or may appear as a separate word after the option. For example, -foo+:... specifies that the completed option and argument will look like either -fooarg or -foo arg.

-optname=
The argument may appear as the next word, or in same word as the option name provided that it is separated from it by an equals sign, for example -foo=arg or -foo arg.

Comment on lines +25 to +29
if is-at-least 5.2; then
_arguments_options=(-s -S -C)
else
_arguments_options=(-s -C)
fi
Copy link
Member Author

@ee7 ee7 Aug 8, 2022

Choose a reason for hiding this comment

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

From man zshcompsys:

The options of _arguments have the following meanings:

-s
Enable option stacking for single-letter options, whereby multiple single-letter options may be combined into a single word. For example, the two options -x and -y may be combined into a single word -xy. By default, every word corresponds to a single option name (-xy is a single option named xy).
Options beginning with a single hyphen or plus sign are eligible for stacking; words beginning with two hyphens are not.

Note that -s after -- has a different meaning, which is documented in the segment entitled `Deriving spec forms from the help output'.

-C
Modify the curcontext parameter for an action of the form ->state. This is discussed in detail below.

-S
Do not complete options after a -- appearing on the line, and ignore the --. For example, with -S, in the line

foobar -x -- -y

the -x is considered an option, the -y is considered an argument, and the -- is considered to be neither.

fi

local line
local curcontext="$curcontext"
Copy link
Member Author

Choose a reason for hiding this comment

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

From man zshcompsys:

The option -C tells _arguments to modify the curcontext parameter for an action of the form ->state. This is the standard parameter used to keep track of the current context. Here it (and not the context array) should be made local to the calling function to avoid passing back the modified value and should be initialised to the current value at the start of the function:

local curcontext="$curcontext"

Copy link
Member

@ErikSchierboom ErikSchierboom left a comment

Choose a reason for hiding this comment

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

Just tested it and it works nicely!

@ee7 ee7 merged commit e4c8643 into exercism:main Aug 13, 2022
@ee7 ee7 deleted the completion-add-zsh branch August 13, 2022 11:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

completions: add zsh
2 participants