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

Default suggestions list has priority over custom list #20

Closed
owlpaste opened this issue Aug 12, 2020 · 7 comments · Fixed by #23
Closed

Default suggestions list has priority over custom list #20

owlpaste opened this issue Aug 12, 2020 · 7 comments · Fixed by #23

Comments

@owlpaste
Copy link

We wanted to use different terminology than offered by default, so created our own inclusive-words.json

It contains same word but different suggetion, running the rule on a testing file still shows up original suggestions instead of our own.

If I rename the path of our inclusive-words.json the process fails and tells me an error that the file cannot be found, also adding new words which are not in the custom list does mean that they get caught and highligted, however it looks like priority of of suggestions are given to the default list, not the one that gets passed in.

Our inclusive-words.json

{
    "words": [
        {
            "word": "blacklist",
            "suggestion": "denylist",
            "explanation": "To convey the same idea, consider '{{suggestion}}' instead of '{{word}}'."
        },
        {
            "word": "whitelist",
            "suggestion": "allowlist",
            "explanation": "To convey the same idea, consider '{{suggestion}}' instead of '{{word}}'."
        },
        {
            "word": "master",
            "suggestion": "main",
            "explanation": "Instead of '{{word}}', you can use '{{suggestion}}'."
        },
        {
            "word": "slave",
            "suggestion": "secondary",
            "explanation": "Instead of '{{word}}', you really should consider an alternative like '{{suggestion}}'."
        }
    ],
    "allowedTerms": []
}

The new rule is added to the task list in this manner in package.json

lint:inclusive": "eslint --rulesdir \"tools/eslint-rules\" --no-ignore --plugin \"inclusive-language\" --rule \"inclusive-language/use-inclusive-words: [warn, tools/eslint-rules/inclusive-words.json]\"",
@github-actions
Copy link

Welcome! 👋

Thank you for posting this issue. 🙇🏼‍♂️ As I am currently on vacation I will come back to you begin of August. Stay safe!

@Charlslum
Copy link

Thanks

@muenzpraeger
Copy link
Owner

Will be fixed with v2.0.0. Sorry for the delay. Busy times.

@owlpaste
Copy link
Author

owlpaste commented Apr 1, 2021

Hello @muenzpraeger, sorry for the delay as well, this was fixed however was broken again by 351923d#diff-7ccc9ad62fc85133a987ff24abfd2c3ae9ea9cbce2e10576d65775130383723dR136

That changed the order of the merge list once more to the original and custom rules no longer have priority :(
I have used tag 2.0.0 though and it works fine, but not in latest release.

@muenzpraeger
Copy link
Owner

@owlpaste Thanks for reaching out, I'll double-check.

Although the linked diff is outdated, as the mechanisms changed to using a map, where the custom config takes precendence to reflect the priority. See here.

@owlpaste
Copy link
Author

owlpaste commented Apr 1, 2021

Yes saw that, it that wasn't in 2.1.0 release though 😄

When that comes out, I will try it. In the mean time, very happy to use 2.0.0 (though lintStrings would have been awesome, il wait)

@muenzpraeger
Copy link
Owner

Yup, but it's in 2.1.1 ;-)

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.

3 participants