Shell completions for Cargo #1646

yodaldevoid · 2019-02-08T16:40:45Z

This is a cleanup of a prior PR (#1425) by @ricvelozo.

Adds the ability to output shell completions for cargo using the command rustup completions <shell> cargo
Updates the completion instructions to mention and suggest placing bash completions in the home directory.

kinnison · 2019-02-16T21:33:22Z

Thanks for your contribution. Could you possibly tidy up the commit sequence (e.g. fold the fixes to formatting into the requisite commits instead of having a 'run rustfmt' commit) to make it easier to review?

I've put this PR on my list to watch but I'd like to see things a little tidier before I review.

I look forward to your changes.

D

yodaldevoid · 2019-02-16T21:59:38Z

Hello @kinnison

Thanks for getting to looking over this. I've tidied up the rustfmt commit, but no other commits jumped out at me as being appropriate to tidy up. Is there anything else you would like cleaned up?

I should note that I wanted to keep @ricvelozo's commits mostly intact. They could get merged together with other commits, to condense everything, but I wanted to preserve his contribution.

kinnison · 2019-02-17T08:22:57Z

Fair enough, I can appreciate wanting to maintain that. I'll put this on my list to review as soon as I can (likely within a day or two)

bors · 2019-02-26T20:49:01Z

☔ The latest upstream changes (presumably #1630) made this pull request unmergeable. Please resolve the merge conflicts.

yodaldevoid · 2019-02-28T02:30:24Z

@kinnison Just wanted to ping you to make sure this hasn't fallen by the wayside.

kinnison

First up, thank you for submitting a PR and contributing to rustup.

This is not a bad PR, though there are a number of points which need cleanup. The big thing is that there's a distinct lack of tests.

Could we please have some basic tests for the new command you're adding?

Thanks,

D.

README.md

src/rustup-cli/rustup_mode.rs

ehuss · 2019-02-28T22:05:56Z

FWIW, I'm proposing a new approach for completions for cargo at rust-lang/cargo#6645. It may not have any impact here, but just wanted to make you aware.

yodaldevoid · 2019-02-28T23:00:55Z

@ehuss I'll keep an eye on that discussion, and when/if it gets implemented if no one else does it I'll make sure to submit a PR moving to the new system for newer versions of cargo.

dwijnand · 2019-03-01T08:29:32Z

Thanks, @ehuss, I've been meaning to cross link that here.

bors · 2019-03-15T20:44:26Z

☔ The latest upstream changes (presumably #1704) made this pull request unmergeable. Please resolve the merge conflicts.

Signed-off-by: Gabriel Smith <ga29smith@gmail.com>

This "command" argument allows rustup to output the completion information of more than just rustup. In this case it adds a cargo completion script. Signed-off-by: Gabriel Smith <ga29smith@gmail.com>

This allows the cargo script for the current default toolchain to be called, and if the script is ever updated the user won't have to maually update their completion script after updating Rust. Signed-off-by: Gabriel Smith <ga29smith@gmail.com>

Signed-off-by: Gabriel Smith <ga29smith@gmail.com>

This removes the duplication between the `variants` and `from_str` implementations.

yodaldevoid · 2019-03-30T07:07:42Z

@kinnison Sorry about the long time since the last update. As the famous quote goes, "Life... finds a way to always get in the way of projects."

I believe I have addressed all requested changes. Please let me know if there is anything else you would like modified.

Moves the error reporting from the completion function to the standard error handling mechanism. CompletionCommand had to be made fully public, not that this matters for a binary target. That said, this could have been prevented by transforming the CompletionCommand into its String form at the error site rather than at the reporting site. It was chosen to go with this method as stringly-typed interfaces are generally bad.

Compares two different command invocations and panics if their return code and stdout/stderr don't match.

kinnison · 2019-03-30T18:07:01Z

Thank you for making the updates, I'll endeavour to review this, likely tomorrow.

yodaldevoid · 2019-04-12T22:41:11Z

@kinnison I hate to be a hypocrite in this regard, but I wanted to ping you about this PR as it has been some time.

kinnison · 2019-04-14T15:48:13Z

@kinnison I hate to be a hypocrite in this regard, but I wanted to ping you about this PR as it has been some time.

No, that's very fair, I needed a kick up the backside it seems the past two weeks have been busy and somehow this slipped off my TODO without me noticing :(

kinnison · 2019-04-14T15:52:27Z

This looks reasonable to me, I'm grateful for the tests too. I wonder if the user ought to be told about how the completions script they're given may vary with the default toolchain; but ultimately I think that's not worth blocking this for.

References: rust-lang/rustup#1646 Signed-off-by: Fletcher Nichol <fnichol@nichol.ca>

pickfire · 2019-07-09T13:59:28Z

Are there fish completions yet?

tesuji · 2019-07-09T14:05:08Z

If you want to contribute, please help with rust-lang/cargo#6645 .

yodaldevoid force-pushed the shell-completions branch from e843bf1 to 49626f9 Compare February 16, 2019 21:50

yodaldevoid force-pushed the shell-completions branch from 49626f9 to da9db14 Compare February 28, 2019 02:21

yodaldevoid force-pushed the shell-completions branch from da9db14 to e80e3a5 Compare February 28, 2019 02:40

kinnison suggested changes Feb 28, 2019

View reviewed changes

README.md Show resolved Hide resolved

src/rustup-cli/rustup_mode.rs Outdated Show resolved Hide resolved

src/rustup-cli/rustup_mode.rs Outdated Show resolved Hide resolved

ricvelozo and others added 9 commits March 16, 2019 14:47

Suggest home directory for bash completion

8371c20

Include shell completion for Cargo

147f46d

Update the README

50c0f3e

Move completion command handler to separate function

4ab5ff2

Signed-off-by: Gabriel Smith <ga29smith@gmail.com>

Add optional "command" argument to rustup completion

e8ade2c

This "command" argument allows rustup to output the completion information of more than just rustup. In this case it adds a cargo completion script. Signed-off-by: Gabriel Smith <ga29smith@gmail.com>

Add information to the help output about bash completions

dc6baeb

Signed-off-by: Gabriel Smith <ga29smith@gmail.com>

Print an error message for unsupported cargo completions

af17f18

Use term2 for completion script outputs

4a4b260

yodaldevoid force-pushed the shell-completions branch from e80e3a5 to eeb1d4c Compare March 16, 2019 20:49

Hold all programs with completions in an array

02d63fc

This removes the duplication between the `variants` and `from_str` implementations.

yodaldevoid force-pushed the shell-completions branch from eeb1d4c to 02d63fc Compare March 20, 2019 04:26

yodaldevoid added 3 commits March 30, 2019 03:45

test: mock: Add expect_ok_eq function

9bb56d0

Compares two different command invocations and panics if their return code and stdout/stderr don't match.

test: Added tests for shell completion script support

0312b11

yodaldevoid force-pushed the shell-completions branch from eed1d01 to 0312b11 Compare March 30, 2019 07:45

kinnison approved these changes Apr 14, 2019

View reviewed changes

kinnison merged commit ed08330 into rust-lang:master Apr 14, 2019

bors mentioned this pull request Apr 14, 2019

Add documentation for installing rustup completion as non root user #1705

Closed

yodaldevoid deleted the shell-completions branch April 14, 2019 16:52

sondr3 mentioned this pull request Apr 23, 2019

rustup: 1.17.0 -> 1.18.1 NixOS/nixpkgs#60098

Merged

10 tasks

ehuss mentioned this pull request Apr 25, 2019

rustup completions don't include completions for Cargo rust-lang/cargo#5596

Closed

fnichol added a commit to fnichol/bashrc that referenced this pull request Apr 30, 2019

Add Cargo completions via Rustup

135ffa4

References: rust-lang/rustup#1646 Signed-off-by: Fletcher Nichol <fnichol@nichol.ca>

NoahGorny mentioned this pull request Dec 5, 2020

Add completion for cargo. Bash-it/bash-it#1717

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Shell completions for Cargo #1646

Shell completions for Cargo #1646

yodaldevoid commented Feb 8, 2019

kinnison commented Feb 16, 2019

yodaldevoid commented Feb 16, 2019

kinnison commented Feb 17, 2019

bors commented Feb 26, 2019

yodaldevoid commented Feb 28, 2019

kinnison left a comment

ehuss commented Feb 28, 2019

yodaldevoid commented Feb 28, 2019

dwijnand commented Mar 1, 2019

bors commented Mar 15, 2019

yodaldevoid commented Mar 30, 2019

kinnison commented Mar 30, 2019

yodaldevoid commented Apr 12, 2019

kinnison commented Apr 14, 2019

kinnison commented Apr 14, 2019

pickfire commented Jul 9, 2019 •

edited

Loading

tesuji commented Jul 9, 2019

Shell completions for Cargo #1646

Shell completions for Cargo #1646

Conversation

yodaldevoid commented Feb 8, 2019

kinnison commented Feb 16, 2019

yodaldevoid commented Feb 16, 2019

kinnison commented Feb 17, 2019

bors commented Feb 26, 2019

yodaldevoid commented Feb 28, 2019

kinnison left a comment

Choose a reason for hiding this comment

ehuss commented Feb 28, 2019

yodaldevoid commented Feb 28, 2019

dwijnand commented Mar 1, 2019

bors commented Mar 15, 2019

yodaldevoid commented Mar 30, 2019

kinnison commented Mar 30, 2019

yodaldevoid commented Apr 12, 2019

kinnison commented Apr 14, 2019

kinnison commented Apr 14, 2019

pickfire commented Jul 9, 2019 • edited Loading

tesuji commented Jul 9, 2019

pickfire commented Jul 9, 2019 •

edited

Loading