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

feat(lint): --rules print all rules #20256

Merged
merged 3 commits into from Aug 27, 2023
Merged

feat(lint): --rules print all rules #20256

merged 3 commits into from Aug 27, 2023

Conversation

sigmaSd
Copy link
Contributor

@sigmaSd sigmaSd commented Aug 23, 2023

The motivation is If I'm using deno lint --rules, I want to see all the rules especially the one that have no tags, since the recommend ones are already active

This pr also prints the tags associated with the rule inline

image

Side question: we can filter with --rules-tags, but what If I want to see only the rules with no tags (not possible currently)

@bartlomieju bartlomieju added this to the 1.37 milestone Aug 24, 2023
Comment on lines 231 to 236
print!(" - {}", rule.code());
if rule.tags().is_empty() {
println!();
} else {
println!(" [{}]", rule.tags().join(", "))
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add some flair here? Maybe put the rule name in colors::cyan and all the remaining info (tag, help and link) in colors::gray?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like this now
image

Comment on lines +207 to +211
let lint_rules = if maybe_rules_tags.is_none() {
rules::get_all_rules()
} else {
rules::get_filtered_rules(maybe_rules_tags, None, None)
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how to handle filtering on rules that have no tags nicely here. Any suggestions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the easiest way would be to add something like --untagged-only

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is an implementation example of untagged-only ba76af6

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a fan of adding a specialized flag. Maybe we could support empty valuer in --rules-tags that would filter rules with no tags?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

--rules-tags="" feels unintuitive to me

Maybe Its better if we leave it as is for now

If you still think it's a good idea I can implement it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't feel strongly about it. We can leave it for a follow up

Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @sigmaSd!

@bartlomieju bartlomieju merged commit 916ddce into denoland:main Aug 27, 2023
13 checks passed
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

Successfully merging this pull request may close these issues.

None yet

2 participants