Add note about not being a general spell checking tool.#1535
Conversation
|
Out of interest @tir38 what was missing to make it a general spell checking tool? What makes you think it isn't one? |
|
I tried to test this by just barfing rando text into a file (e.g. |
|
It looks for common spelling mistakes rather than membership in a complete dictionary. Maybe it can be rephrased to take this into account? |
|
@larsoner yep, that was what I hoped to accomplish by updating the README |
Co-authored-by: Eric Larson <larson.eric.d@gmail.com>
| It does not check for word membership in a complete dictionary, but instead | ||
| looks for a set of common misspellings. Therefore it might catch errors like | ||
| "adn", but it will not catch "adnasdfasdf". | ||
|
|
There was a problem hiding this comment.
Derp. Talk about circular dependency. Using Codespell to check readme of repo finds spelling error, which only exists to show example of spelling error. Not sure if I should fix this.
There was a problem hiding this comment.
Well at least we know it's working @tir38 ! Joking apart we could either ignore the line or add the specific typo to an allowed list (although if we do the latter it would be better if it was a more obscure word/less likely typo, but perhaps that defeats the point of the example; abandonned is our goto typo, but it's not as obvious as the example one).
@larsoner Travis didn't catch this as it's only checking the module source stuff, not the surrounding files:
codespell --skip="codespell_lib/tests/test_basic.py,codespell_lib/data/*" codespell_lib/
There was a problem hiding this comment.
Feel free to add it to the checks
|
Sorry this slipped by the wayside @tir38 . I think it would still make sense to get it in. To avoid the error it's now trapping (due to #1596), we'll need to skip it, the safest way is probably adding an exclude file into |
peternewman
left a comment
There was a problem hiding this comment.
Sorry this slipped by the wayside @tir38 . I think it would still make sense to get it in, do you want to have a go. I think the following changes should do the trick.
| It does not check for word membership in a complete dictionary, but instead | ||
| looks for a set of common misspellings. Therefore it might catch errors like | ||
| "adn", but it will not catch "adnasdfasdf". | ||
|
|
Co-authored-by: Peter Newman <peternewman@users.noreply.github.com>
Don't spellcheck README.md
peternewman
left a comment
There was a problem hiding this comment.
Sorry @bl-ue , we can't just skip the whole of README.rst because of one (deliberate) typo! How daft is codespell going to look when it ships a readme with typos in it?!?
The ideal solution is to check readme separately and ignore just the one or two deliberate typos (via ignore words). The more general solution for other people where it's less critical would be to ignoreit across the entire codebase. We should really use the ideal solution given what we're representing!
|
Are you happy to re-engineer the CI tests then after this @bl-ue , or do you want me to? |
I'm sorry! That's what we get with my eager merging 😝😆
You mean update the CI to only ignore the specific typos in the readme? Sure, I'll do it! |
😄
The gold-plated solution would probably be best, skip the readme as you've done in the main run, but add another run which checks just the readme but ignores the additional deliberate typos, then we won't get "adn" all over the rest of the codebase. |
|
Good idea. I'll do it 👍🏻 |
|
thanks y'all! |
|
No problem @tir38 — thank you for evaluating codespell! I'd recommend some other tools that are probably more what you're thinking of when you hear spellchecker: |
|
Hahaha — sorry @peternewman! 😂 |
I spent way too much time figuring this out the hard way.