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

zsh completion script for rustup is broken #2268

Open
Aloxaf opened this issue Mar 18, 2020 · 5 comments
Open

zsh completion script for rustup is broken #2268

Aloxaf opened this issue Mar 18, 2020 · 5 comments
Labels

Comments

@Aloxaf
Copy link

Aloxaf commented Mar 18, 2020

Problem

#2031 add a special argument +toolchain, this breaks the zsh completion script generated by clap.

Detail
+toolchain is treated as a positional argument by clap, this brings two problems:

  1. rustup \t will also generate file list because +toolchain is treated as a file.
  2. the first argument is occupied by +toolchain, so the completion script thinks the real subcommand starts from the 2nd argument. This means only rustup xxx toolchain \t will generate completion for toolchain subcommand.

Steps

  1. generate the completion script: rustup completions zsh > a_dir_in_fpath/_rustup
  2. type rustup \t, you will see both rustup commands and local files (they shouldn't exist) in candidates.
  3. type rustup toolchain \t, you will still see rustup commands, not rustup toolchain commands.

Possible Solution(s)

  1. Throw this issue to clap
  2. Use a (semi-)hand-written completion script
  3. Do a search-and-replace in clap's output

Notes

Output of rustup --version: rustup 1.21.1 (2020-02-23)
Output of rustup show:

Default host: x86_64-unknown-linux-gnu
rustup home:  /home/aloxaf/.rustup

installed toolchains
--------------------

stable-x86_64-unknown-linux-gnu
nightly-2019-12-20-x86_64-unknown-linux-gnu
nightly-x86_64-unknown-linux-gnu (default)

installed targets for active toolchain
--------------------------------------

wasm32-unknown-unknown
x86_64-pc-windows-gnu
x86_64-unknown-linux-gnu

active toolchain
----------------

nightly-x86_64-unknown-linux-gnu (default)
 rustc 1.43.0-nightly (d3c79346a 2020-02-29)
@Aloxaf Aloxaf added the bug label Mar 18, 2020
@Aloxaf Aloxaf changed the title zsh completion script is broken zsh completion script for rustup is broken Mar 18, 2020
@Aloxaf
Copy link
Author

Aloxaf commented Mar 18, 2020

A temporary fix for those who need it:

--- _rustup     2020-03-18 19:08:18.865256496 +0800
+++ _rustup_fix 2020-03-18 18:42:23.837856740 +0800
@@ -23,16 +23,18 @@
 '--help[Prints help information]' \
 '-V[Prints version information]' \
 '--version[Prints version information]' \
-'::+toolchain -- release channel (e.g. +stable) or custom toolchain to set override:_files' \
+'(+beta +nightly)+stable[use the stable toolchain]' \
+'(+stable +nightly)+beta[use the beta toolchain]' \
+'(+stable +beta)+nightly[use the nightly toolchain]' \
 ":: :_rustup_commands" \
 "*::: :->rustup" \
 && ret=0
     case $state in
     (rustup)
-        words=($line[2] "${words[@]}")
+        words=($line[1] "${words[@]}")
         (( CURRENT += 1 ))
-        curcontext="${curcontext%:*:*}:rustup-command-$line[2]:"
-        case $line[2] in
+        curcontext="${curcontext%:*:*}:rustup-command-$line[1]:"
+        case $line[1] in
             (dump-testament)
 _arguments "${_arguments_options[@]}" \
 '-h[Prints help information]' \

@kinnison
Copy link
Contributor

Hi, so in theory we need to work out whether (a) clap needs updates and if so what, and (b) whether rustup needs updates in order to support completion better.

Do you have any experience with clap and completions generation? If we could tell clap enough that it'd be able to at least say this field only makes sense if it starts with a + then that could help completions behave properly.

@Aloxaf
Copy link
Author

Aloxaf commented Mar 23, 2020

Sorry, I don't know much about them.

To be honest, I'm not confident with let clap generate the correct completion automatically.
+toolchain is a very special argument. If it is a normal option like +stable or +nightly, we can just let clap support +option ( like -option ). But it is +<toolchain> with <toolchain> as a parameter.

Ummm, I can't come up with a perfect way to solve it. I prefer to something like .ignore_completion() to just let clap ignore it. At least this can make completion works well in most cases. A better and more complex way is to support regex as a validator, which is easier to turn to *sh code than rust function.

Just personal opinion.

@kinnison
Copy link
Contributor

I don't really want rustup to have to carry custom patched completions, so really it's up to clap to offer us something. Ideally clap would offer custom completion function support which added hidden CLI arguments which could be used to add completion support in a more dynamic fashion.

@kinnison
Copy link
Contributor

Looks like clap-rs/clap#1232 is the issue to track for this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants