Conversation
bnjbvr
left a comment
There was a problem hiding this comment.
Thanks, looks good. One small comment and one question, and this should be good to go!
src/main.rs
Outdated
| if !args.json { | ||
| eprintln!("Analyzing dependencies of crates in this directory..."); | ||
| } | ||
| args.paths.push(PathBuf::from(".")); | ||
| } else { | ||
| } else if !args.json { |
There was a problem hiding this comment.
Are these if !args.json required? The fact the tool is outputting with eprintln means this is going to stderr, and a user emitting JSON would likely be fine piping from stdout, so I think we could keep the stderr output as is. Thoughts?
There was a problem hiding this comment.
I went ahead and made this change (and have pushed the commits), however I think we want to go back to something like I had originally. Look at: https://doc.rust-lang.org/std/macro.eprintln.html
Use eprintln! only for error and progress messages. Use println! instead for the primary output of your program.
When not in --json mode, the primary output is what it used to print, and so that should use println!. So even
eprintln!("Analyzing dependencies of crates in this directory...");Should change to use just println! according to that.
Now, if we do that, we cannot output both JSON and plain text, so I think I'll need to make a change to make it either or.
- If
--jsonflag is on, then it will only print JSON to stdout (usingprintln!). - Else, then it will only print the plain text it previously printed to stdout (using
println!).
I will wait to hear back before I make this change though.
|
|
||
| if args.version { | ||
| println!("{}", env!("CARGO_PKG_VERSION")); | ||
| eprintln!("{}", env!("CARGO_PKG_VERSION")); |
There was a problem hiding this comment.
Note: Printing the version should go to stdout, so this change is wrong and will need to be reverted one way or another in this PR.
Fixes #217.