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

[spell-check] Fixes for a possible patch release #1173

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

savetheclocktower
Copy link
Contributor

A couple fixes need to be applied to spell-check; this is the PR that will hold them. Opening as draft for now; I'll commentate as I add commits.

@savetheclocktower
Copy link
Contributor Author

First we need to fix CI; there's a failing test in the spell-check specs. This test passed for me on macOS, but is failing on Linux, so I can only assume that the Linux dictionary considers foo to be a word, and macOS’s dictionary doesn't. We'll soon see if my theory is correct!

@savetheclocktower
Copy link
Contributor Author

This commit removes the console.log I left in inadvertently… and also applies some small fixes to eliminate some ESLint gripes with the specs.

@savetheclocktower
Copy link
Contributor Author

Finally, I'm probably going to argue for reverting the change we made to the default spell-check.grammars setting. Here's why:

  • There's something a bit dumb about this package: whenever an edit is made and the editor has stopped changing for a short period of time, the entire document is re-checked. This is rather costly when the document is large (like a several-thousand-line–long source code file). The spell-checking happens in a background thread, but if you're making frequent changes to a certain file, the checking happens often, and it's CPU-intensive. It could be quite a battery drain.
  • So at the very least it could be argued that adding source comment was premature, and that it would be better to revert this change and instead add the ability for spell-check to check/recheck the buffer incrementally. This is worth doing even independent of our decision with this setting.
  • Still, after having used this feature for a few days, I'm completely on the fence about whether it's desirable for the average user. It has helped me spot a few typos in my code comments, but it also complains about things that aren't words — like camelCasedMethodNames — and those aren't actionable. It also complains about file extensions — so if I make reference to a highlights.scm file in a code comment, the scm has a red squiggle underneath.

For now I'd like to revert the change to spell-check.grammars setting, and then we can have the debate later on when spell-check is a bit more considerate of the user's system resources. If a user still wants it, they can add the value back in the package settings — the part that enables selective buffer checking isn't going anywhere.

…of the `"source comment"` value.

A user can add it back in if they want spell-checking of code comments, but it's probably a bit too opinionated to be the default.
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 this pull request may close these issues.

1 participant