coreutils
coreutils copied to clipboard
Remove "spell-checker:ignore" lines
Some files have a spell-checker:ignore line (that can be huge sometimes). I guess some editor uses this. However it's cumbersome to maintain these while doing code refactors, etc. and they're annoying while grepping the code. My hope is that these lines are obsolete and can be removed, or is anyone relying on them?
Is there any reason not to remove the spell-checker lines?
I think we should remove them. I haven't added them to any new files I've created anyways, so I think we should just remove them from all files to be consistent. Also, they'll probably become obsolete as the project develops.
@rivy from looking at the git blame you seem to have added those lines. Are they still useful to you?
No, we shouldn't currently remove them.
The exceptions are still needed to keep a handle on spelling in the code base. Otherwise, there are just an overwhelming number of "false positive" spelling errors to visually parse when looking for "true" spelling errors. Even with the lack of current spelling maintenance and the resulting "false positives", I can see at least 20 spelling errors, typos, and inappropriate words in the current code base (using a quick cspell --config .vscode\cSpell.json **/*.rs).
I'll probably also be adding an automatic spell-check to the "Style" portion of the CICD in the future as a "soft" failure... when time is available and the code base settles a bit.
Using the VSCode "Code Spell Checker" extension (aka, 'streetsidesoftware.code-spell-checker') will point them out as you're coding to minimize inefficiencies... I'll add that extension to the "recommended" list to prompt for folks to use it. I can move some of the exceptions to the config file or specific exception dictionaries. And I'll move to do that with some other maintenance on the exception lines when I get a bit of time.
I'll try to include most commonly used odd words in a workspace exception list. But, I do think that some effort to include spelling exceptions on the developer side is a good thing as it should encourage better comment and naming hygiene.
Would it be possible to add those allowed words to a global file to the repo, instead of to each file individually? Otherwise, if we want to require this, it should be checked by CI that these lines are always up to date and added to every new file.
@miDeb , certainly!
I made some progress toward that kind of consolidation in earlier commits (see cSpell.json). But I got bogged down by the sheer weight of all the words and changes while simultaneously dealing with the time crunch that arose for me with this last year's pandemic craziness. Hence all the spell-checker:ignore (ToDO) ... lines.
I'm trying to strike a balance between including reasonable jargon, name, people, and project specific words vs pushing developers to produce readable (and correctly spelled) code and comments. I have developed some expanded word lists/dictionaries (see dxx wordlists). I'm working on pulling them together for a commit here (hopefully in the next few days). I'll try to work through some of the "ToDO" spell exceptions at the same time.
I do want to add a CICD Style spelling check (with soft failure), but it has languished on the lower levels of my priority list (PRs welcome :smile:).
Hopefully, with that global pool of words and CI warnings, developers could then either fix spelling "faults" or add their exceptions as spell-checker:ignore ... lines. That would make it fairly straight-forward to review those spelling exceptions at later dates and refactor when needed.
Sound reasonable?
Sounds good! I'll try and investigate how we could integrate spellcheck to CI. If there's anything else where I could help, please let me know.
Additionally, spell checking is not only an advantage for clean code, but in our case we also get to spell-check the outputs of utilities.
A good portion of the work to move the spell checking more centrally is now done in PR #2315.
I'll start work on more of the ToDO spell checker exceptions next week.
Feedback welcome...
I'm happy with the spell checking right now. It also make sense to have some exceptions specified per file. Can we close this?
I see no objections since Jan 19, so closing this :)