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

Names of missing required arguments are always colorized #775

Closed
dotdash opened this issue Dec 12, 2016 · 4 comments
Closed

Names of missing required arguments are always colorized #775

dotdash opened this issue Dec 12, 2016 · 4 comments

Comments

@dotdash
Copy link
Contributor

dotdash commented Dec 12, 2016

Affected Version of clap

2.19.2

Expected Behavior Summary

Coloring is only applied when output is sent to a terminal.

Actual Behavior Summary

The name of the missing required argument is always colorized, giving messed up output e.g. capturing output into vim's quickfix buffer.

|| [cargo run --bin render]                                                                                                                  
|| unused variable: `matches`, #[warn(unused_variables)] on by default                                                                       
||    |                                                                                                                                      
|| 33 |     let matches = App::new("My  app")                                                                                                
||    |         ^^^^^^^                                                                                                                      
||-                                                                                                                                          
||      Running `target/debug/render`                                                                                                        
|| The following required arguments were not provided:                                                                                       
||     ^[[1;31m<CONFIG>^[[0m                                                                                                                 
||-                                                                                                                                          
|| USAGE:                                                                                                                                    
||     render <CONFIG> <WIDTH> <HEIGHT>                                                                                                      
||-                                                                                                                                          
|| For more information try --help                                                                                                           
|| [Finished in 1 seconds with code 1]              
@kbknapp
Copy link
Member

kbknapp commented Dec 18, 2016

Is this with using AppSettings::ColorNever?

@dotdash
Copy link
Contributor Author

dotdash commented Dec 19, 2016

It's not, but even with it, that one thing is still colored.

@dotdash
Copy link
Contributor Author

dotdash commented Dec 19, 2016

OK, this is because the closure in validate_required directly uses Format::Error instead of the Colorizer and thus unconditonally emits ANSI color codes. The quick fix would be to create a Colorizer there, and using its error() method instead. Is that an acceptable approach (I'm not familiar with the code base)?

@kbknapp
Copy link
Member

kbknapp commented Dec 19, 2016

Great catch! Yeah that would be an acceptable fix. I'm on mobile right now, so if you'd like to do the PR feel free 😉

homu added a commit that referenced this issue Dec 19, 2016
Only colorize missing arguments when appropriate

Instead of using Format::Error directly, we need to use a Colorizer to
make sure that the message only gets colorized when it is appropriate
to do so.

Fixes #775
homu added a commit that referenced this issue Dec 19, 2016
Only colorize missing arguments when appropriate

Instead of using Format::Error directly, we need to use a Colorizer to
make sure that the message only gets colorized when it is appropriate
to do so.

Fixes #775
kbknapp pushed a commit that referenced this issue Dec 19, 2016
Instead of using Format::Error directly, we need to use a Colorizer to
make sure that the message only gets colorized when it is appropriate
to do so.

Fixes #775
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants