-
Notifications
You must be signed in to change notification settings - Fork 63
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: pull diagnostics #153
Conversation
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.
Very cool :) Great work!!
Mostly Nits except for the coment regarding the Severity
🙌🏻
|
||
let errors = diagnostics | ||
.iter() | ||
.filter(|d| d.severity() == Severity::Error) |
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.
There's also Severity::Fatal
which would not be included here, should we maybe check d.severity() >= Severity::Error
?
@@ -34,7 +34,7 @@ pub(crate) struct JunitReporterVisitor<'a>(pub(crate) Report, pub(crate) &'a mut | |||
|
|||
impl<'a> JunitReporterVisitor<'a> { | |||
pub(crate) fn new(console: &'a mut dyn Console) -> Self { | |||
let report = Report::new("Biome"); | |||
let report = Report::new("PgLsp"); |
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.
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.
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.
hahaha
// pub only: Vec<RuleSelector>, | ||
// pub skip: Vec<RuleSelector>, |
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.
Should we remove those?
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.
we need them once the linter is there
Co-authored-by: Julian Domke <[email protected]>
Co-authored-by: Julian Domke <[email protected]>
Co-authored-by: Julian Domke <[email protected]>
Co-authored-by: Julian Domke <[email protected]>
adds diagnostics
Copied and adapted from Biome. The CLI-related stuff will need some in-depth testing and a number of improvements eg to just handle sql files. I also do not know yet how to define a glob pattern. The
std-in
mode is also not implemented yet. But the basics are working, and we will figure out the ergonomics later when the linter is implemented.check
commandfor now just for syntax errors but it will be easily extendable. will add the linter after this.