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

feat(dict): allow support of multi dictionaries and min length #2

Merged
merged 1 commit into from
Jan 23, 2017

Conversation

yannickglt
Copy link
Contributor

Allow to add dictionaries to the spell checker:

new JsSpellChecker({
  dicts: [
    {
      aff: __dirname + '/../node_modules/lintspelljs/dicts/en_US.aff',
      dic: __dirname + '/../node_modules/lintspelljs/dicts/en_US.dic'
    },
    __dirname + '/../dicts/english.dic',
    __dirname + '/../dicts/jetbrains.dic'
  ]
});

And allow to pass words less than a minimum length:

new JsSpellChecker({
  minLength: 4
});

With this parameter, words with a length less than minLength will not be checked. It's useful for single-letter words.

@yannickglt
Copy link
Contributor Author

Is there a chance to merge this pull request @aotaduy?

@aotaduy
Copy link
Owner

aotaduy commented Oct 3, 2016

Thanks for the PR! give me some time to review it.

@yannickglt
Copy link
Contributor Author

Thanks a lot! I am not sure about the way to import dictionaries. Reading the file from at the beginning might be simpler and more testable.
For example:

new JsSpellChecker({
  dicts: [
    {
      aff: fs.readFileSync(__dirname + '/../node_modules/lintspelljs/dicts/en_US.aff'),
      dic: fs.readFileSync(__dirname + '/../node_modules/lintspelljs/dicts/en_US.dic')
    },
    fs.readFileSync(__dirname + '/../dicts/english.dic'),
    fs.readFileSync(__dirname + '/../dicts/jetbrains.dic')
  ]
});

@aotaduy
Copy link
Owner

aotaduy commented Oct 19, 2016

Hello, I'm ready to merge this one. I like this one more than the other (#3) since I'm not sure about the capitalize functionality.
Can you tell me what would be the case to use the capitalization.

Best Regards

@yannickglt
Copy link
Contributor Author

Hey!
I don't think capitalization is relevant anymore. I removed the pull request.
Thank you.

@aotaduy aotaduy merged commit 92ce1fa into aotaduy:master Jan 23, 2017
@aotaduy
Copy link
Owner

aotaduy commented Jan 23, 2017

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 this pull request may close these issues.

2 participants