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

Only correct "they are" if it isn't at the end of a sentence #40

Merged
merged 11 commits into from
Feb 20, 2022

Conversation

boardfish
Copy link
Contributor

Thanks to @Spone for this.

In a sentence like "We appreciate all contributions, no matter how small they are.", Vale (backed by this repo) would suggest replacing they are with they're, resulting in the slightly awkward "no matter how small they're." This PR aims to fix that with a negative lookahead that ensures "they are" isn't followed by a period marking the end of the sentence.

Do we also want to include other symbols that can end sentences (? !) in the regex?

Co-authored-by: Hans Lemuet <hans@spone.net>
@jdkato
Copy link
Member

jdkato commented Feb 17, 2022

Could you add a few tests to Contractions/test.md?

Essentially just sentences that should either pass or fail the rule.

@boardfish
Copy link
Contributor Author

@jdkato I've added tests. There's a small catch - I was unable to get it to error if a sentence ends with a contraction.

I've also realized that most of the other mappings listed would probably have valid sentences that can trigger issues, so I'll work through those too.

@boardfish
Copy link
Contributor Author

The tests marked with a TODO comment don't yet pass. I'll read around, but if there's anything anyone knows about how Vale handles regex that's stopping me from using those positive lookaheads, please let me know 🙏

@jdkato jdkato merged commit 7bcc6b2 into errata-ai:master Feb 20, 2022
@jdkato
Copy link
Member

jdkato commented Feb 20, 2022

Everything looks okay on my end -- the TODOs are passing for me:

Screen Shot 2022-02-20 at 1 58 34 PM

So, I'm going to go ahead and merge this.

Thanks for your work here!

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