-
Notifications
You must be signed in to change notification settings - Fork 497
Revert adding lien
to the rare dictionary
#3631
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
Revert adding lien
to the rare dictionary
#3631
Conversation
Since `lien` is a word frequently used when working with the Google Terraform Provider, it feels wrong to treat it as a typo. Humbly suggesting to revert that change This reverts commit 536ccb5. Fixes codespell-project#3630
Note that this typo is in the rare dictionary. Perhaps the rare dictionary should not be selected by default. |
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.
Can you move these to the code dictionary please, which seems like the right place to me given they upset programming related text, but are still potentially valid/useful outside of that domain.
107c1a1
to
e41cf86
Compare
e41cf86
to
300afdc
Compare
lien->line
"lien
to the code dictionary
Moved now |
@peternewman @nikolaik It seems to me that |
I agree with you @DimitriPapadopoulos but no strong opinion, so open to go either way |
When you say specific domain do you mean Google Terraform? Isn't that the same as say:
Which is only relevant to Assembler (AFAIK), to pick probably one of the more obscure ones? |
From my perspective it's not particularly related to programming, it's a legal term that just so happens to be used by Google Cloud Platform. Whilst it's that usage that has resulted in this debate, fundamentally it's just a word, and one I (native British English speaker) was familiar with long before I came across it's usage in GCP. |
300afdc
to
107c1a1
Compare
From what I'm hearing in this discussion and #3630 is that adding a well defined word with current use as a misspelling would not make sense and do more harm than good. Originally added in #3460 as a misspelling with the justification "Found in Emacs." I changed this PR back back to a plain revert, since that now seems like the best outcome, to me. |
lien
to the code dictionary lien
to the rare dictionary
In the long term, I would rather keep rare words in the rare dictionary, that's what it's for. Perhaps the rare dictionary shouldn't be selected by default. I'll settle for removing this entry for now. Thank you @nikolaik. Just noticed another possible alternative. From the README:
Perhaps we should disable automatic fixes by adding a comment to all or part of the words in the rare dictionary. Example: $ cat DICTIONARY.txt
lien->line, actually a word in the OED and SCOWL (And Friends)
$
$ cat test.txt
lien
$
$ codespell -w -D DICTIONARY.txt test.txt
test.txt:1: lien ==> line | actually a word in the oed and scowl (and friends)
$
$ codespell -D DICTIONARY.txt test.txt
test.txt:1: lien ==> line | actually a word in the oed and scowl (and friends)
$ |
Thanks Dimitri! |
Autofixing cases which rely on a person using their discretion, where there is ambiguity (dictionary definition exists) sounds wrong to me. |
So would you agree to reintroducing |
I'm not in favour of reintroducing lien, for the reasons mentioned above. Though for existing fixes in the rare dictionary that have dictionary definitions, I would consider it, since that would be less of a breaking change for users. I would like to add that one of the reasons this project appeals to me is that it provides an good out of the box experience, with defaults that fixes common mistakes, does not give false positives, and requires little intervention from users to give value. I use codespell via the pre-commit hook and one of the sources of friction are when a mistake has multiple possible fixes (ambiguous) which I don't mind. Though disabling autofix for fixes in the rare dictionary, that are part of the default experience, will add more friction, maybe too much. |
So removing the rare dictionary from the default dictionaries would be the best option, wouldn't it? |
I can't say, it depends a lot on the contents of the rare dictionary, but it might |
|
Since
lien
is a frequently used legal term and also happens to be used when working with the Terraform GoogleTerraform resource
google_resource_manager_lien
, it feels wrong to treat it as a typo.Humbly suggesting to revert that change
This reverts commit 536ccb5.
Fixes #3630