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

Anx is a single syllable - issue #46 #47

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

Conversation

RamakrishnanArun
Copy link

Fix for #46

Added a fix for the word anxious which is 2 syllables as you can see from the google search and dictionaries.

https://www.google.com/search?q=anxious
image

and
https://www.merriam-webster.com/dictionary/anxious
https://www.dictionary.com/browse/anxious?s=t

@codecov-io

This comment has been minimized.

1 similar comment
@codecov-commenter

This comment has been minimized.

@codecov-io

This comment has been minimized.

@wooorm
Copy link
Member

wooorm commented Oct 16, 2020

Nice, thanks for working on a fix!

  1. Could you check a couple of other words with those letters?
$ sed -n '/anx/p' < /usr/share/dict/words
anxietude
anxiety
anxious
anxiously
anxiousness
banxring
Caranx
lanx
Manx
Manxman
Manxwoman
overanxiety
overanxious
overanxiously
phalanx
phalanxed
planxty
reanxiety
unanxiety
unanxious
unanxiously
unanxiousness
  1. Could you add the new test cases (as as a couple of the above, and anxious itself) to the javascript tests instead of the fixtures? Like so perhaps:

    syllable/test/index.js

    Lines 235 to 263 in d7f2e84

    t.deepEqual(
    [
    'reminiscense',
    'commonsense',
    'innocense',
    'recompense',
    'condense',
    'defense',
    'dispense',
    'expanse',
    'expense',
    'immense',
    'intense',
    'license',
    'mouthrinse',
    'nonsense',
    'offense',
    'pretense',
    'response',
    'suspense',
    'cleanse',
    'dense',
    'rinse',
    'sense',
    'tense'
    ].map(syllable),
    [4, 3, 3, 3, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 1, 1, 1, 1, 1],
    'GH-39 ([aeiouy]nse$)'
    )

@RamakrishnanArun
Copy link
Author

Yeah, I realised later that some of those words didn't match, like anxiety for example. Looking at that now.

@RamakrishnanArun
Copy link
Author

This is proving to be a little more tricky than I thought. I will step through the code a little more.

@wooorm
Copy link
Member

wooorm commented Oct 17, 2020

Hmm, yeah this can be tricky, because of so many weird regex interplay. Syllables are hard!

If there is no way around it, problematic.json could be an escape hatch, but as that’s not scalable, code is preferred

@RamakrishnanArun
Copy link
Author

I want to avoid adding it to the problematic.json file. There are other similar words like society that might benefit from these patterns too.

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