rust-lang / rustup

The Rust toolchain installer
https://rust-lang.github.io/rustup/
Apache License 2.0
6.16k stars 885 forks source link

Duplicative auto completions #2268

Open Aloxaf opened 4 years ago

Aloxaf commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

Looks like https://github.com/clap-rs/clap/issues/1232 is the issue to track for this

rami3l commented 6 days ago

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

Currently cargo has started using this feature:

This would be interesting to look at.