-
Notifications
You must be signed in to change notification settings - Fork 249
feat: fix "I walks" / "he walk" etc. #2333
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: fix "I walks" / "he walk" etc. #2333
Conversation
elijah-potter
left a comment
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.
This looks like very good work. Unfortunately, I found a few false positives that I'd love to see turn into passing tests:
- "On April 10th, I suggested she consider a smaller, more intimate gathering."
- "I suggested she sell it and use the proceeds to help with her relocation expenses, or perhaps rent a similar camera while in Barcelona."
- "I’ve offered her alternative options – suggesting she rent a similar cottage herself, or even offering to cover part of the cost – but she always deflects, insisting she deserves to use this cottage."
- "He donned his heavy oilskins and descended the winding staircase, his boots echoing in the hollow tower."
Excellent finds! Quite surprising that none of the texts I tested on used these very common constructions. I supposed it's more literary than what's in GitHub readmes. The first three all involve a verb which makes the following "that" optional. Another one I could think of is "insisted". There are probably others that will reveal themselves. The last one is an example of one of the words which is not an irregular verb but which has its forms spread over multiple |
…onoun-verb-agreement-233
…etrail/harper into pronoun-verb-agreement-233
|
I've found a few more false-positives. I think there may something more primitive at play here that can't be addressed by adding exceptions.
|
Yeah exceptions are kind of a last resort. Looking at this now... each is for a different reason:
|
…etrail/harper into pronoun-verb-agreement-233
|
I'm very sorry to keep pushing this back. We're getting very close, and I really want this rule to be excellent, particularly because it is so helpful. I was only able to find three more false-positives.
I suspect these are simply small problems in the dictionary. |
No apologies needed! This is the best way - using real-world data to find the edges you can't anticipate. The middle one is easily the hairiest for us! "lose you points" where "to lose" is a ditransitive verb that takes two objects. The first object, "you", is a pronoun which doesn't change form for subject vs object (like "it" but unlike every other personal pronoun). "Points" is here a plural noun but in form is also a 3rd person singular present verb". The 2nd person pronoun + 3rd person verb "you points" is the kind of thing this kind of linter looks for. Actually it wasn't too hard. We can be sure there will be persistent edge cases. So put it through the wringer. Look for false negatives as well as false positives - but that is more difficult. |
…onoun-verb-agreement-233
This fixes 1. and 3. from the new PR review. But this does not yet tackle the ditransitive verb issue for 2.
22f9a71 to
68b60bf
Compare
Issues
Implements the first part of #233
Description
Fixes disagreement of grammatical number between pronoun and verb:
I walks → I walk
He walk → He walks
In the course of implementing this I found many annotations missing from the dictionary.
I also added a bunch of missing irregular verbs.
I added a couple of methods to
SequenceExprbut the main one would be the new constructorSequenceExpr::with()which is a short way to expressSequenceExpr::default().then()I propagated
::with()through all the other linters I could find appropriate for it.Demo
How Has This Been Tested?
Checklist