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

Support for tab completion #360

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ pub enum Command {
setting = structopt::clap::AppSettings::DontCollapseArgsInUsage
)]
Release(ReleaseOpt),
Completions(CompletionsOpt),
Comment on lines 17 to +18
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How come you did it at this level? I'm assuming most people will be running cargo-release as cargo release and not cargo-release.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put it at the top-level since that's how rustup and a few other tools in the Rust packaging/toolchain ecosystem do it. My argument for that would be that cargo release is how people should be running it interactively, but cargo-release completions [...] makes more sense in the context of a shell profile (to emphasize that the completions are coming from just cargo-release and that they aren't hooked up to the cargo completions yet).

}

#[derive(Debug, StructOpt)]
Expand Down Expand Up @@ -222,3 +223,9 @@ fn resolve_bool_arg(yes: bool, no: bool) -> Option<bool> {
(_, _) => unreachable!("StructOpt should make this impossible"),
}
}

#[derive(StructOpt, Debug, Clone)]
pub struct CompletionsOpt {
#[structopt(long, short = "s", possible_values = &clap::Shell::variants(), case_insensitive = true)]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any particular reason you did case insensitive?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, no particular reason. If I had to contrive one, people might do --shell=Bash or similar and reasonably expect that to work. But I don't mind changing it to case sensitive at all, to keep the possibilities simpler.

pub shell: clap::Shell,
}
31 changes: 20 additions & 11 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
use std::collections::HashMap;
use std::collections::HashSet;
use std::ffi::OsStr;
use std::io::Write;
use std::io::{self, Write};
use std::path::Path;
use std::process::exit;

Expand All @@ -27,16 +27,25 @@ use crate::replace::{do_file_replacements, Template};
use crate::version::VersionExt;

fn main() {
let args::Command::Release(ref release_matches) = args::Command::from_args();

let mut builder = get_logging(release_matches.logging.log_level());
builder.init();

match release_workspace(release_matches) {
Ok(code) => exit(code),
Err(e) => {
log::error!("Fatal: {}", e);
exit(128);
match args::Command::from_args() {
args::Command::Release(ref release_matches) => {
let mut builder = get_logging(release_matches.logging.log_level());
builder.init();

match release_workspace(release_matches) {
Ok(code) => exit(code),
Err(e) => {
log::error!("Fatal: {}", e);
exit(128);
}
}
}
args::Command::Completions(ref completion_matches) => {
args::Command::clap().gen_completions_to(
"cargo-release",
completion_matches.shell,
&mut io::stdout(),
);
Comment on lines +44 to +48
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How should we document this capability, including giving the example code you had in the R?

We could probably put it in the long help for the command.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, perhaps an FAQ entry? The example I included in the top comment is extremely specific to bash, so including it in the long help might confuse users of other shells (zsh, fish, etc.)

We could also do it in the "reference" docs, although those seem to cover just the release subcommand so perhaps not...

}
}
}
Expand Down