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

Add zsh and bash completions for x #107827

Closed
wants to merge 1 commit into from

Conversation

clubby789
Copy link
Contributor

Suggest the x subcommands, then suggest the paths they may be invoked with. We can then point users to these scripts in the dev guide. There is probably a better way to implement these, but I'm unfamiliar with shell completion APIs.

The content of src/etc/completions/*_paths is from the output of x <cmd> -h -v, hardcoded to avoid the shell having to invoke x whenever the completion script is loaded.

@rustbot label +T-bootstrap

@rustbot
Copy link
Collaborator

rustbot commented Feb 9, 2023

r? @Mark-Simulacrum

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Feb 9, 2023
@jyn514
Copy link
Member

jyn514 commented Feb 9, 2023

The content of src/etc/completions/*_paths is from the output of x <cmd> -h -v, hardcoded to avoid the shell having to invoke x whenever the completion script is loaded.

Hmm, I don't know how I feel about this. I'd like to have an upgrade path in case we add more paths at a later date.

cc @thomcc, I think you had some plans around this.

@clubby789
Copy link
Contributor Author

I did wonder if a tidy check might help with this. Alternatively, just changing the argument to use more basic patterns might make it simpler to maintain, as well as running a bit faster - tab completing the tests here is noticably slower than the default shell completion as it processes the globs.

@Mark-Simulacrum
Copy link
Member

I don't think we should be creating lists like this - part of the reason we aim for paths to be the inputs is that shells already autocomplete those.

Can we generate these on the fly perhaps? It seems like e.g. a cache based on git HEAD might work okay here and not cause too many problems.

At minimum, if we do need hardcoded lists, they should be verified and bless'able like formatting.

@thomcc
Copy link
Member

thomcc commented Feb 12, 2023

For fish-shell I started on this but never got around to completing it. That said, for fish a hard-coded path list wasn't needed, I just parsed the list that was in help output. It was fairly ad-hoc but worked. In general I figured that since not many people use fish-shell, the only way it would be upstreamable is if it were small and not problematic.

@jyn514
Copy link
Member

jyn514 commented Feb 12, 2023

another alternative is to autocomplete only the subcommands (build/test/...) and treat any following arguments as a path for the purposes of completion.

cc #107375 which proposed switching to clap, that would autogenerate these completions for us.

@clubby789
Copy link
Contributor Author

Resolving as a path would probably be the easiest and quickest way to do this. Do shells have the ability complete files relative to the source root though? Otherwise tab completion wouldn't work from a subdirectory, which was the main motivation for doing it this way

@jyn514
Copy link
Member

jyn514 commented Feb 13, 2023

Can you spawn a subshell and generate completions from there? not sure if there's a way to pass env variables back to the parent shell but you could do COMPREPLY=(cd $(git rev-parse --show-toplevel) && ...) I think.

@clubby789
Copy link
Contributor Author

This seems to be working better. Bash completion isn't great for large amounts of files (fills the screen and partially overwrites its own output) but I'm not sure if that's an issue with the completion or bash just not handling it well

@Mark-Simulacrum
Copy link
Member

Bash completion isn't great for large amounts of files (fills the screen and partially overwrites its own output) but I'm not sure if that's an issue with the completion or bash just not handling it well

This seems pretty unfortunate. Maybe we can limit to a subset (e.g., head -n20) or something?

Are these completions enabled by default for x? What do users need to do to enable them?

@albertlarsan68
Copy link
Member

Is it possible to also generate/have completions for NuShell please?

@clubby789
Copy link
Contributor Author

Are these completions enabled by default for x? What do users need to do to enable them?
Enabling completions will need users to run a command to load the completions for the current shell instance, or added to bash/zshrc (which we can mention in the dev guide)

I was hoping to replace this with #108083, since clap_complete automatically generates completions (and there seems to be an extension for NuShell) but some bugs may need to be fixed in clap_complete to work with the layout of xs arguments

@Mark-Simulacrum
Copy link
Member

@rustbot author

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 4, 2023
bors added a commit to rust-lang-ci/rust that referenced this pull request May 14, 2023
Generate shell completions for bootstrap with Clap

Now that rust-lang#110693 has been merged, we can look at generating shell completions for x.py with `clap_complete`. Leaving this as draft for now as I'm not sure of the best way to integration the completion generator. Additionally, the generated completions for zsh are completely broken (will need to be resolved upstream, it doesn't seem to handle subcommands + global arguments well).
I don't have Fish installed and would be interested to know how well completions work there.

Alternative to rust-lang#107827
@clubby789
Copy link
Contributor Author

Closed by #111388

@clubby789 clubby789 closed this May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants