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

Fixed Edgecase Exclusion List Caveat #19

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

OneTrueC
Copy link

@OneTrueC OneTrueC commented May 5, 2024

Not the most elegant solution, but it works.

Not the most elegant solution, but it works
@christoomey
Copy link
Owner

Hey @OneTrueC - thanks for putting this up.

I'd like to understand the intention of the changes better before merging in. It looks like there are two different cases you're handling, one for preserving UPPERCASE and the other for only matching the whole word for exclusion words. Is that right?

If so, I'm wondering if you can share some specific cases for these?

@OneTrueC
Copy link
Author

OneTrueC commented May 5, 2024

The caveat in the readme states that the exclusion list shouldn't be ignored and uses all caps for examples, so I made my changes under the assumption that the intended behavior was to check both for the beginning and end.

As for examples:

  • the fanciful tales of HTML -> The Fanciful Tales of HTML
  • (given that thoughtbot is in the exclusion list) secrets of thoughtbot -> Secrets of thoughtbot

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.

2 participants