-
-
Notifications
You must be signed in to change notification settings - Fork 449
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
feat: flag to suppress existing diagnostics #4008
base: main
Are you sure you want to change the base?
Conversation
I would appreciate a look from a maintainer here to see if I'm on the right track architecturally. I'm not completely sure that I have the right idea. I was following what looks like the strategy for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely take a look at ematipico's comment here: #4007 (comment)
crates/biome_cli/src/commands/mod.rs
Outdated
// TODO: The flag requires a value here. Am I able to let them pass `--write-suppressions`, | ||
// without a value? Option<Option<String>> wasn't it. | ||
#[bpaf(long("write-suppressions"))] | ||
write_suppressions: Option<String>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found this: pacak/bpaf#383
It seems like some kind of custom Parser is required, but I'd prefer if someone more familiar with bpaf weighed in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to be able to pass both --write-suppressions
to get Some("default_val")
and --write-suppressions=something
to get Some("something")
then yea, what you found is the right value.
Make a function fn write_suppressions() -> impl Parser<Option<String>>
with code from the last comment (for bar
), and do something like
#[bpaf(external)]
write_suppressions: Option<String>,
I'd prefer if someone more familiar with bpaf weighed in.
👀
TL;DR from Ema's comment: I wasn't even close on my first attempt. 😄 Circling back. |
Well, it at least works now. 😄 Maintainer questions:
My to-dos:
|
} | ||
|
||
match params.fix_file_mode { | ||
FixFileMode::ApplySuppressions => { | ||
// No-op, handled above |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, this isn't great.
Summary
Working on #4007 to create a flag for writing ignores.
Test Plan
WIP