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

Make plural after a number lower case #82

Merged
merged 2 commits into from
May 25, 2019
Merged

Make plural after a number lower case #82

merged 2 commits into from
May 25, 2019

Conversation

robwebset
Copy link
Contributor

Pull request to support adding the 's' as lower case if ending in a numeric value.

Solves request #77

Thanks
Rob

@coveralls
Copy link

coveralls commented Nov 13, 2017

Coverage Status

Coverage decreased (-0.7%) to 99.346% when pulling 01cb5fd on robwebset:master into 9be3a26 on blakeembrey:master.

@coveralls
Copy link

coveralls commented Nov 13, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 1688d09 on robwebset:master into 9be3a26 on blakeembrey:master.

@robwebset
Copy link
Contributor Author

OK, we should be good now - first error was for spaces/tabs , the second was me adding a test into the test.js

Thanks
Rob

@blakeembrey
Copy link
Collaborator

How about just inverting the current checks to do word.toLowerCase() === word to check it "did not" upper-case? That would skip the need for checking numbers and other related cases.

@coveralls
Copy link

coveralls commented Nov 14, 2017

Coverage Status

Coverage decreased (-0.7%) to 99.338% when pulling a8e9a22 on robwebset:master into 9be3a26 on blakeembrey:master.

@robwebset
Copy link
Contributor Author

I tried that, unfortunately it doesn't work with things like "Work Order2" and "SoundFX2"

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling a8e9a22 on robwebset:master into 9be3a26 on blakeembrey:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling a8e9a22 on robwebset:master into 9be3a26 on blakeembrey:master.

@coveralls
Copy link

coveralls commented Nov 14, 2017

Coverage Status

Coverage decreased (-0.7%) to 99.338% when pulling 0a74b5f on robwebset:master into 9be3a26 on blakeembrey:master.

@robwebset
Copy link
Contributor Author

Sorry, I'm an idiot! It does work - Corrected it now :)

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.7%) to 99.338% when pulling 3d750d0 on robwebset:master into 9be3a26 on blakeembrey:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.7%) to 99.338% when pulling 3d750d0 on robwebset:master into 9be3a26 on blakeembrey:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.7%) to 99.342% when pulling 8e1dade on robwebset:master into 9be3a26 on blakeembrey:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.7%) to 99.342% when pulling 8e1dade on robwebset:master into 9be3a26 on blakeembrey:master.

@coveralls
Copy link

coveralls commented Nov 15, 2017

Coverage Status

Coverage decreased (-23.03%) to 76.974% when pulling aaa5b0e on robwebset:master into 9be3a26 on blakeembrey:master.

@coveralls
Copy link

coveralls commented Nov 15, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling ab66aac on robwebset:master into 9be3a26 on blakeembrey:master.

@coveralls
Copy link

coveralls commented Nov 15, 2017

Coverage Status

Coverage remained the same at 100.0% when pulling 8242cee on robwebset:master into f663b1d on blakeembrey:master.

@robwebset
Copy link
Contributor Author

OK, sorted everything out now ... hopefully!

@@ -49,7 +49,10 @@
// Tokens are an exact match.
if (word === token) return token;

// Upper cased words. E.g. "HELLO".
// Lower cased words. E.g. "hello".
if (word === word.toLowerCase()) return token.toLowerCase();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Is it possible to remove the title case check now though and fallback to it by default if neither of these cases match? No need to check if that should be the default now anyhow.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if we did that things like soundFX, oDonald etc would fail

@wernerm
Copy link

wernerm commented Jan 30, 2018

Why is this not being merged?

@blakeembrey blakeembrey merged commit acb79de into plurals:master May 25, 2019
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.

4 participants