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

Include 'an's along with 'a's as an article #19

Open
dhruvkb opened this issue Jan 10, 2021 · 3 comments
Open

Include 'an's along with 'a's as an article #19

dhruvkb opened this issue Jan 10, 2021 · 3 comments

Comments

@dhruvkb
Copy link
Contributor

dhruvkb commented Jan 10, 2021

Problem

In many places the indefinite article 'a' is manipulated, either by stripping it away or treating it as 1. The article 'an' should be processed in the same way.

In all these regexes a should be replaced with an? to cover both articles.

Occurrences

string.gsub!(/\ba$/, '') && string.rstrip! # doesn't make sense for an 'a' at the end to be a 1

string.gsub!(/(^|\W)\ba\b(?=$|\W)(?! (?:#{ALL_ORDINALS_REGEX}))/i, '\1<num>' + 1.to_s)

string.gsub!(/(^|\W)\ba\b(?=$|\W)/i, '\1<num>' + 1.to_s)

Resolution

I could make a PR if there's consensus that this needs to be fixed.

@jduff
Copy link
Owner

jduff commented Jan 18, 2021

Can you give a few examples of the test cases you would make pass with this change? I can't think of the cases where you would have an instead of a and want it treated the same way. I also haven't spent much time with this codebase in awhile now so may just be missing the context for this.

@dhruvkb
Copy link
Contributor Author

dhruvkb commented Jan 20, 2021

Sure, I can give some examples.

Words Numerized
an eighth 1/8
an eleventh 1/11

You wouldn't use "a" as the article on words starting with vowels.

@jduff
Copy link
Owner

jduff commented Jan 29, 2021

I forgot about fractions lol.

If you want to submit a PR for this I'll take a look, thanks!

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 a pull request may close this issue.

2 participants