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

Quiet mode for all cargo commands #1748

Merged
merged 1 commit into from Jul 1, 2015

Conversation

Projects
None yet
7 participants
@pwoolcoc
Copy link
Contributor

pwoolcoc commented Jun 25, 2015

New version of #738, adds -q and --quiet to all cargo subcommands.

cc @sinistersnare (sorry it took me 1.5 months to circle back around to this)

@rust-highfive

This comment has been minimized.

Copy link

rust-highfive commented Jun 25, 2015

r? @huonw

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

@huonw

This comment has been minimized.

Copy link
Member

huonw commented Jun 25, 2015

@rust-highfive rust-highfive assigned alexcrichton and unassigned huonw Jun 25, 2015

@sinistersnare

This comment has been minimized.

Copy link

sinistersnare commented Jun 25, 2015

Great! :) I have been busy with an internship for a bit so I have forgotten about this. Thanks for CCing me!

I wonder if panicking is the correct thing to do when both --quiet and --verbose are specified. Should we just default to one if both are specified?

@pwoolcoc

This comment has been minimized.

Copy link
Contributor Author

pwoolcoc commented Jun 25, 2015

Maybe it should default to Normal if both are passed? That is, passing both is the same as passing neither?

@pwoolcoc

This comment has been minimized.

Copy link
Contributor Author

pwoolcoc commented Jun 25, 2015

(also, it seems like Travis is complaining because rustup is broken on osx?)

pub fn set_verbosity(&mut self, verbose: bool, quiet: bool) {
self.verbosity = match (verbose, quiet) {
(true, true) => {
panic!("You cannot set both --verbose and --quiet");

This comment has been minimized.

@alexcrichton

alexcrichton Jun 26, 2015

Member

Cargo in general tries to not panic as much as possible, can this function return a CargoResult which is then handled through the normal means?

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Jun 26, 2015

Thanks! The travis failure looks benign, but can you also add some tests for this as well?

cc @wycats

@pwoolcoc pwoolcoc force-pushed the pwoolcoc:feature/quiet-mode branch from 2e714e2 to ce2fe70 Jun 29, 2015

@pwoolcoc

This comment has been minimized.

Copy link
Contributor Author

pwoolcoc commented Jun 29, 2015

@sinistersnare

This comment has been minimized.

Copy link

sinistersnare commented Jun 29, 2015

(Im not a reviewer, but) it looks good, thanks :)

}

#[derive(Debug)]
pub struct VerbosityError;

This comment has been minimized.

@alexcrichton

alexcrichton Jun 29, 2015

Member

One-off errors like this can typically be created with human("cannot set both --verbose and --quiet")

_ => {
Normal
}
};

This comment has been minimized.

@alexcrichton

alexcrichton Jun 29, 2015

Member

This would be a little more concise and exhaustive as:

self.verbosity = match (verbose, quiet) {
    (true, true) => return Err(human("...")),
    (true, false) => Verbose,
    (false, true) => Quiet,
    (false, false) => Normal,
};
@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Jun 29, 2015

Can you add tests for:

  • Running a cargo command with -q
  • Running a cargo command with -q and -s.

Ah, also cc @rust-lang/tools

@pwoolcoc

This comment has been minimized.

Copy link
Contributor Author

pwoolcoc commented Jun 30, 2015

@alexcrichton pushed changes, thanks for the suggestions.

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Jul 1, 2015

Thanks! Could you also squash the commits together? Other than that I think this is good to go.

Closes #653
Adds a `-q|--quiet` option to all cargo commands that suppresses output from
cargo. In the case of `cargo run`, output from the project executable will
still be shown.
@pwoolcoc

This comment has been minimized.

Copy link
Contributor Author

pwoolcoc commented Jul 1, 2015

What? You don't want to merge 15 commits? :)

Will push the squashed commit momentarily.

@pwoolcoc pwoolcoc force-pushed the pwoolcoc:feature/quiet-mode branch from 2de75b1 to 88e557f Jul 1, 2015

@alexcrichton

This comment has been minimized.

Copy link
Member

alexcrichton commented Jul 1, 2015

@bors: r+ 88e557f

Thanks!

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jul 1, 2015

⌛️ Testing commit 88e557f with merge b48cd46...

bors added a commit that referenced this pull request Jul 1, 2015

Auto merge of #1748 - pwoolcoc:feature/quiet-mode, r=alexcrichton
New version of #738, adds `-q` and `--quiet` to all `cargo` subcommands.

cc @sinistersnare (sorry it took me 1.5 months to circle back around to this)
@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jul 1, 2015

@bors bors merged commit 88e557f into rust-lang:master Jul 1, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.