prql
prql copied to clipboard
Add color option to CLI
Currently the CLI can't take options such as --color / --no-color. Ideally we'd be able pass these, and they would flow through with an Options struct to the compiler.
A future version could check whether we're in a TTY and default to --no-color if so (it's also possible that clap offers something like this?).
Labeling as Good first issue (which also make good nth issues). As ever, lots of folks will be happy to answer questions!
Hi Max,
I'd like to work on this. My thought of the steps for adding the option are:
-
Add one
enable_colorboolean field in the Option struct under thesql/mod.rs. -
Add the cli argument
enable_color,color, orno-colorto the compile command. -
Pass the cli argument as an
Optionfield to the compile function
Please correct me if my understanding is wrong.
That would be great @Shuozeli !
I think that sounds very reasonable.
I would pass a full Options struct to the compile function (I think that's what you were suggesting). And yes re the --color / --no-color flags.
There's a "smaller" version where we strip out the color at the CLI level, I think that would be easier but less good (but OK!)
@aljazerzen lmk if you have any thoughts!
Hmm, sql does not do any highlighting ATM, so it doesn't need color option (at least for now). But the cli argument would be use to control this param in error formatting. This function is called
The best way to do this would be:
- add a new Options struct to
lib.rs:struct Options { sql: sql::Options, color: bool } - change
compile(prql: &str, options: Option<sql::Options>)to thiscompile(prql: &str, options: Option<Options>) - add a
impl Default for Optionsandimpl From<sql::Options> for Optionsfor convenience - make sure that all crates compile and pass
cargo test
OK, so we'd have a nested Options struct. That seems very reasonable too.
@Shuozeli does that make sense to you?
Yes, thank you for the clarification. I will be working on this one.
@Shuozeli Any progress on this?