harper-core: flag 'few' in more time contexts#2660
harper-core: flag 'few' in more time contexts#2660LouisLau-art wants to merge 2 commits intoAutomattic:masterfrom
Conversation
hippietrail
left a comment
hippietrail
left a comment
There was a problem hiding this comment.
So cool to see someone working on this! I made the original few_units_of_time_ago linter before I added match_to_lint_with_context()
| | "your" | ||
| | "his" | ||
| | "her" | ||
| | "their" |
There was a problem hiding this comment.
See if .is_determiner() on TokenKind works here.
There was a problem hiding this comment.
It's a method on TokenKind, which is accessed from the .kind field of the Token. Note that my/our/your/his/her/their are considered "possessive determiners" so should be included. If not there's a method to check for those too. Did you leave out "its" intentionally?
| // inserting an article would be incorrect. | ||
| if matches!( | ||
| prev_word.as_deref(), | ||
| Some("too" | "very" | "so" | "quite" | "how") |
There was a problem hiding this comment.
See if .is_degree_adverb() on TokenKind works here.
There was a problem hiding this comment.
I think this PR is close, but I am also interested if that method would work instead. @LouisLau-art, would you give it whirl?
|
Thanks for the context — really appreciate you building the original linter! I used the existing behavior as a baseline and tried to keep the new context checks minimal + well-covered by tests. |
elijah-potter
left a comment
elijah-potter
left a comment
There was a problem hiding this comment.
@LouisLau-art, I'd love to get this merged. Would you give that method a whirl and remove the parenthesized bit at the end of the description? Thanks!
hippietrail
left a comment
hippietrail
left a comment
There was a problem hiding this comment.
I added some thoughts to my previous review.
| | "your" | ||
| | "his" | ||
| | "her" | ||
| | "their" |
There was a problem hiding this comment.
It's a method on TokenKind, which is accessed from the .kind field of the Token. Note that my/our/your/his/her/their are considered "possessive determiners" so should be included. If not there's a method to check for those too. Did you leave out "its" intentionally?
3 participants
Closes #1114.
What changed
FewUnitsOfTimeAgorule from only matchingfew <time unit> agoto matchingfew <time unit>and then using surrounding context to decide whether it should be corrected toa few.After few minutes ...andFew minutes after ....Heuristics (intentionally conservative)
Triggers when
few <time unit>is followed byago/after/before/lateror preceded byafter/before/in/within/for/since.Skips when it already has an article (
a/an/the) or whenfewis clearly negative (too/very/so/quite/how few).Tests
after few minutes/few minutes after.the few ...,too few ...,past few ...).