-
Notifications
You must be signed in to change notification settings - Fork 100
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: "news" used as plural #691
base: master
Are you sure you want to change the base?
Conversation
Addresses Automattic#407 Because of many lower/upper case variants this and similar might be better as a dedicated lint - or `matcher.rs` could be changed to handle case variants
Turning into a draft so this doesn't get merged before I fix it. I didn't think of the false positive "Back in the days of ..." but too busy to switch branches. |
I love the additions. Would you mind making them "real" rules in For example: create_linter_map_phrase!(
News,
ExactPhrase::from_phrase("many news are"), // Source pattern is mapped to the
"a lot of news is", // target string
"MESSAGE",
"DESCRIPTION"
); If you'd like to go a bit further, you can optionally use the |
On a tangent, why does this code ... create_linter_map_phrase!(LoAndBehold, ExactPhrase::from_phrase("long and behold"), "lo and behold", "Did you mean `lo and behold`?", "Detects the exact phrase `long and behold` and suggests replacing it with the idiomatically correct `lo and behold`"); ...n ot trigger the precommit error for having the error in its description? I just had to change the description of the modal-of lint to not have |
I just put in a bunch of time on the plural news one and it turned out to be much more combinatorial and contextual than I expected. I think a dedicated linter will be needed for it: how many news are shown → ✅how much / ✅how many |
Addresses #407
Because of many lower/upper case variants this and similar might be better as a dedicated lint - or
matcher.rs
could be changed to handle case variants