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

Update zsh completion file #606

Merged
merged 11 commits into from
Mar 17, 2020
Merged

Update zsh completion file #606

merged 11 commits into from
Mar 17, 2020

Conversation

heyrict
Copy link
Contributor

@heyrict heyrict commented Mar 13, 2020

I just modified the clap-generated completion file for zsh to make it working.

Note that completion for --set calls just --evaluate to get the variables, so it can slow down the console a bit if you have heavy evaluation logic.

heyrict and others added 3 commits March 13, 2020 15:25
I just modified the clap-generated completion file for `zsh` to make it working.

Note that completion for `--set` calls `just --evaluate` to get the variables, so it can slow down the console a bit if you have heavy evaluation logic.
@casey
Copy link
Owner

casey commented Mar 14, 2020

Thank you for doing this!

I'd like to avoid running just --evaluate during completions. If I add functionality to just to print out a space-separated list of variable names, would that allow removing the call to just --evaluate, in favor of just --variables? For example:

x := "hello"
y := "bye"
$ just --variables
x y

The existing ZSH completions file is auto-generated, so to merge this I'll have to write a little code inside of the just --completions subcommand which modifies the generated zsh completions file with the additions in this PR. That will be easier if the modifications are simple insertions and deletions. So, for example, avoiding modifying or reformatting the block that starts with:

local context curcontext="$curcontext" state line
    _arguments "${_arguments_options[@]}" \

Would be ideal, and the fewer changes to the generated existing code, the easier that will be. Would it be possible to refactor this to introduce slightly less churn to the generated code?

@casey
Copy link
Owner

casey commented Mar 14, 2020

I just landed #608, which adds a just --variables subcommand that prints out a space-separated list of variable names. Unfortunately, this also caused the generated completion scripts to change, which introduced conflicts with this PR.

@heyrict
Copy link
Contributor Author

heyrict commented Mar 14, 2020

Oh, sorry. I thought the completion files are maintained manually since they are added to VCS. Should I modify the bin/generate-completions script instead? I'm not familiar with editing files with command line except with vi -c.

Anyway, here are the list of generated _arguments that should be changed:

  • '-s+[..]' should be changed to '-s+[..]: :_just_commands', so is with --show=[..]
  • '*--set=[..]' should be changed to '*--set[..]: :_just_variables'
  • Fallback to complete files '::ARGUMENTS...:_files' should be moved afterwards and the array should be assigned to a variable $common as it is used twice. I thought this is the hardest part.

@casey
Copy link
Owner

casey commented Mar 14, 2020

Oh, sorry. I thought the completion files are maintained manually since they are added to VCS.

No worries, it's a somewhat convoluted setup. The completion scripts are generated by clap, the command line argument parsing library that just uses. The just binary has a subcommand, just --completions SHELL, which prints the completions from clap to stdout. I also decided to save the completions to the completions directory, so that packagers will be more likely to know that they exist, and won't have to know to run just --completions to get them.

Should I modify the bin/generate-completions script instead? I'm not familiar with editing files with command line except with vi -c.

This is the implementation of just --completions:

fn completions(shell: &str) -> Result<(), i32> {

So I'll have to edit that function to modify the generated completions file with your changes.

I think the best thing to do is to modify this PR to have fewer diffs, and then I'll modify the completions function so that it produces the right output.

For example, there's this diff:

-    local ret=1
+    local context curcontext="$curcontext" state line ret=1
 
     if is-at-least 5.2; then
         _arguments_options=(-s -S -C)
     else
         _arguments_options=(-s -C)
     fi
 
-    local context curcontext="$curcontext" state line

I think these changes could be avoided, since they move around existing code.

Anyway, here are the list of generated _arguments that should be changed:

  • '-s+[..]' should be changed to '-s+[..]: :_just_commands', so is with --show=[..]
  • '*--set=[..]' should be changed to '*--set[..]: :_just_variables'
  • Fallback to complete files '::ARGUMENTS...:_files' should be moved afterwards and the array should be assigned to a variable $common as it is used twice. I thought this is the hardest part.

That makes sense. What I'll probably do is create an arguments_wrapper function that looks like this:

arguments_wrapper() {
  _arguments $@ # pass arguments to _arguments
  common=$@
}

And then replace the call to _arguments with a call to arguments_wrapper, to minimize the amount of code that needs to change.

@heyrict
Copy link
Contributor Author

heyrict commented Mar 15, 2020

@casey Thank you for your response! I've just refactored the PR to include minimal changes to the completion file. Hopefully it will be easier to add.

What I'll probably do is create an arguments_wrapper function that looks like this

Well, sorry I didn't get the point how this function would work. I left the $common variable as it was.

@casey
Copy link
Owner

casey commented Mar 16, 2020

I just pushed some diffs to your branch which modify the completions subcommand, now in src/subcommand.rs, to modify the ZSH completions script that it generates. The completions file shouldn't have changed too much, although I modified it a little bit to make it easier to create.

Is there any way to test ZSH completion scripts?

Also, I've never actually used ZSH completions. How do I tell zsh that I want to load the completion script?

@heyrict
Copy link
Contributor Author

heyrict commented Mar 16, 2020

Thanks for your fast work!

Is there any way to test ZSH completion scripts?

Well, I don't know any automated approach to test a completion script. Most of the time I test the script by inserting lines e.g. echo $somevar >> debug.txt and tail -F debug.txt

How do I tell zsh that I want to load the completion script?

Afaik zsh uses fpath variable to get the location where completion scripts lies. Completion script for just should be placed in {compdir}/_just where {compdir} is a member of fpath.

By convention completion scripts are placed in .zsh/completions or so. I'll use /tmp/completions here to minimize the effect done to your existing configs.

mkdir -p /tmp/completions
cp /path/to/just/completions/just.zsh /tmp/completions/_just

After that, simply paste the following to your zsh or .zshrc will make zsh completion work.

fpath+=/tmp/completions
# The styles for completion descriptions and messages, zsh will hide them by default.
zstyle ':completion:*:descriptions' format "%U%B%d%b%u"
zstyle ':completion:*:messages' format "%F{green}%d%f"
autoload -Uz compinit
compinit -u

@casey casey merged commit e79482f into casey:master Mar 17, 2020
@casey
Copy link
Owner

casey commented Mar 17, 2020

Thanks for your fast work!

You bet!

Well, I don't know any automated approach to test a completion script. Most of the time I test the script by inserting lines e.g. echo $somevar >> debug.txt and tail -F debug.txt

I searched a little bit, and that's my impression too, i.e. that it doesn't seem to be common to test zsh completion scripts. I'd like to add that eventually, since generating completion scripts from clap and then modifying them in code feels error-prone, but that can wait.

After that, simply paste the following to your zsh or .zshrc will make zsh completion work.

Sweet, worked for me. Pretty cool! It's awesome to see the recipes names and variable values complete. Also rad that it prints the whole recipe in green.

Thanks so much for this! Better completions scripts is a frequent request, so this will definitely make zsh users happy.

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.

None yet

2 participants