rnote icon indicating copy to clipboard operation
rnote copied to clipboard

feat: spellcheck for text strokes

Open Kneemund opened this issue 1 year ago • 6 comments

Implements spellcheck for typewriter strokes using the enchant library that wraps around many spellchecking libraries (Hunspell, Aspell, AppleSpell, ...). It is also used by libspelling, the official GTK4 spellchecking library. Switching to a different library (like zspell) should be fairly simple.

The enchant crate for Rust is unfortunately not as good as libspelling's (official) bindings.

  • Closes #271.
  • Uses binary trees for caching results and only checks modified words.

TODO

  • [x] Add per-document settings for enabling/disabling spellcheck and setting the language.
  • [x] Add interface for selecting and applying suggestions/corrections, e.g. a popover menu in the sidebar.
  • [ ] Allow locking spellcheck behind a password (https://github.com/flxzt/rnote/issues/271#issuecomment-1514241105).
  • [ ] Adapt Windows build workflow after deciding which languages to bundle.

Things to Discuss

  • Spellcheck correction picker.
  • Is the locking the spellcheck out of scope?
  • Should you be able to build the CLI without enchant?
  • Two subcrates of icu are added to the dependencies, namely icu_locid and icu_displaynames. They're used to convert the IETF region tags to display names.
  • Evaluate the use of enchant for Windows & MacOS. MacOS might not need much extra work, since the AppleSpell integration should work with the system dictionaries. We need to bundle dictionaries on Windows though - deciding which languages should be bundled sounds tricky.

Kneemund avatar Oct 31 '24 17:10 Kneemund

If the option to lock spellchecking is not out of scope, the UI could look something like this.

grafik

Questions about how to implement it, and maybe more importantly, how to avoid this being a footgun for users, remain. The digital dictionaries I've used in the past just played a fairly loud sound when unlocking the options again. That's not really an option here.

@LeSnake04 do you have any ideas on how to implement this? Admin passwords (which you suggested in your comment) seem like a big footgun to me - they're either ineffective, or can lock users out of this feature forever until they e.g. delete the app settings.


EDIT: I thought about it for a while, and I have an idea that doesn't require passwords.

Locking and unlocking would be possible at all times, but a random number would be displayed in the UI that changes whenever you (re-)lock the spellcheck. That way, teachers can assert that the spellcheck was kept locked at all times by writing down and later comparing the random number. And it's not a footgun for regular users/people who tend to forget passwords.

If anyone is affected by this, it would be nice to know if this would pass the requirements of your institution.

Kneemund avatar Nov 02 '24 23:11 Kneemund

This is what wavy error lines could look like. I'll push these for now, since that's just the standard look. I personally also like the dashed lines though - I'm open to reverting/changing this.

Wavy

grafik

Dashed

grafik

Kneemund avatar Nov 03 '24 13:11 Kneemund

I think that there are mainly two options for displaying spellcheck corrections.

  1. Popup with suggestions for word at cursor.
  2. Panel with suggestions for all spellcheck errors in the current text stroke.

Proof of concept for the first option:

https://github.com/user-attachments/assets/a54f2f28-300f-4f4e-bf47-b2fe24ba628b

The "blinking button" is quite distracting IMO, maybe the sensitivity should always be enabled. Instead of making the button insensitive if there are no suggestions, we could add a hint telling users to move the cursor over a erroneous word.

Kneemund avatar Nov 26 '24 11:11 Kneemund

More fleshed out version without the button sensitivity changing; I personally like this a lot more.

https://github.com/user-attachments/assets/84d227fa-94cb-47a1-b9fb-68f53fc7adb7

Kneemund avatar Nov 27 '24 18:11 Kneemund

I wonder if the requirement of not having spellcheck could be done with a compile feature flag

Doublonmousse avatar Nov 27 '24 20:11 Doublonmousse

That would definitely solve it, but this requirement seems to be quite common, so I think we should try our best to make it accessible.

The feature flag might still be a good idea for the CLI though, it probably shouldn't depend on enchant and its dependencies. But I'm not sure if it's worth the "bad" code (every single EngineView creation would need that attribute - maybe we should turn those into macros 😄). In addition to that, the CLI also depends on ALSA/audio libraries, as the pen sounds feature isn't optional either (not really an excuse here though).

Kneemund avatar Nov 27 '24 20:11 Kneemund