-
-
Notifications
You must be signed in to change notification settings - Fork 118
fix: unknown word report config change #8261 #8273
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
base: main
Are you sure you want to change the base?
Conversation
|
Need any more help? |
Jason3S
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We just need to add some tests:
We will need a fixture:
issue-8261
It can be added to test/index.test.mts. See issue-4870 as an example.
Jason3S
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you.
packages/cspell-eslint-plugin/src/plugin/defaultCheckOptions.cts
Outdated
Show resolved
Hide resolved
Co-authored-by: Jason Dent <[email protected]> Signed-off-by: Bence Márkus <[email protected]>
Signed-off-by: Jason Dent <[email protected]>
Jason3S
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the update.
Did you forget to add the fixtures and the eslint tests?
It would look like this: in src/test/index.test.mts
readFix('issue-4870/sample.js', {
cspell: {
dictionaries: ['business-terms'],
dictionaryDefinitions: [
{
name: 'business-terms',
path: fixtureRelativeToCwd('issue-4870/dictionaries/business-terminology.txt'),
},
],
},
}),| numSuggestions: 8, | ||
| generateSuggestions: true, | ||
| autoFix: false, | ||
| ...defaultCheckOptions, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What was the logic to move this after the explicit values?
I understand that the two should NOT overlap, but if they do, then the result might be unexpected to the person who added the value to defaultOptions because it will get overwritten by the ...defaultCheckOptions.
This
PRcontains the changes required to resolvecspell ESLint plugin: support typos-only reporting (CLI --report typos / unknownWords report-common-typos) #8261