Add a -s/--silent option to silence user-facing messages

[tgross35]

I have a usecase where it would be nice to use cargo-show-asm programmatically, since it does a nicer job of filtering and demangling than most builtin utilities. However, UI messages currently print to stdout, meaning they need to be manually filtered:

$ cargo asm -p libm --lib 2>/dev/null | cat
Try one of those by name or a sequence number
  0 "<f32 as libm::math::support::float_traits::Float>::abs" [9]
  1 "<f32 as libm::math::support::float_traits::Float>::copysign" [18]
  2 "<f32 as libm::math::support::float_traits::Float>::normalize" [21]
  3 "<f64 as libm::math::support::float_traits::Float>::abs" [9]

Introduce a -s/--silent flag that reduces the verbosity level, effectively being the the opposite of -v:

$ path/cargo-asm asm -p libm --lib -s 2>/dev/null | cat
  0 "<f32 as libm::math::support::float_traits::Float>::abs" [9]
  1 "<f32 as libm::math::support::float_traits::Float>::copysign" [18]
  2 "<f32 as libm::math::support::float_traits::Float>::normalize" [21]
  3 "<f64 as libm::math::support::float_traits::Float>::abs" [9]

-s was chosen to mirror Curl, since -q is already used as the flag to quieten Cargo's output.

[pacak]

Hmmm... I intentionally made them stdout so piping to less or to a file produces something readable. For programmatic usage I'd rather add --quiet that would suppress anything non-essential. What do you think?

For that we'd change the default verbosity value to be 1, and make all those prints conditional when verbosity is at or above 1. New verbosity parser can look like this:

fn verbosity() -> impl Parser<usize> {
    let verbose = short('v')
        .long("verbose")
        .help("more verbose output, can be specified multiple times")
        .req_flag(())
        .count();
    let quiet = short('q')
        .long("quiet")
        .help("pls think of something reasonable")
        .req_flag(())
        .count();
    construct!(verbose, quiet).map(|(v, q)| (v + 1).saturating_sub(q))
}

[tgross35]

Sounds reasonable to me 👍 I can do that

[tgross35]

Actually, any thoughts about having -q still print these messages but route them to stderr rather than stdout? They can still be useful, especially errors, so it would be nice for them to show up in logs but just not the tool-captured output. And then if somebody doesn't want these messages at all, they can just discard stderr.

[pacak]

I think currently all the errors go to stderr - if not - that's a bug and -q shouldn't affect it.

[pacak]

Some of the stuff that shows up with verbosity greater than zero are warnings and I think it's okay to keep them in stdout - to make debugging easier.

[tgross35]

Alright, updated with a new flag to reduce verbosity. -q is already taken as the flag that quiets Cargo's output, so I picked -s like Curl uses. Let me know if you would prefer a different letter or if I should combine them somehow.

I think currently all the errors go to stderr - if not - that's a bug and -q shouldn't affect it.

There were a couple instances where this didn't seem to be the case, so added a patch that updates this.

[pacak]

I think you need to drop one of the unused imports, but looks good other than that.

[tgross35]

Some of the stuff that shows up with verbosity greater than zero are warnings and I think it's okay to keep them in stdout - to make debugging easier.

For what it's worth, this is somewhat less convenient for use via tools - if running cargo asm ... | grep some_symbol were to print warnings to stdout, they are lost to grep or, with -s, just never get emitted. So there isn't really a good way to be notified of warnings that might be related to the output. On the flip side, it is easier to combine stdout+stderr with cargo asm ... 2>&1 | less if you want to see everything in one place.

That being said, I understand this is primarily a user-friendly utility and use via scripting isn't really a common usecase, so the changes here work for me 👍 (I just want to be able to have CI automatically generate a diff of changed assembly for specific symbols).

[pacak]

You need to run tests locally and commit changes to the readme

[tgross35]

You need to run tests locally and commit changes to the readme

Sorry, thought I did that. Done.

[pacak]

.args(std::iter::repeat("-v").take(format.verbosity))

Need to reduce this by 1 with saturating sub

[tgross35]

Makes sense, done.

[tgross35]

Thank you for the quick review!

[pacak]

Looks good, thank you for your contribution ❤️

I'll try to make a new release shortly.

While this tool started as a user facing - I'm not opposed to making it easier to use it from scripts, just need to decide what and how...

[pacak]

0.2.46 is out

[tgross35]

Thank you very much!