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

feat: spellcheck for text strokes #1269

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

Kneemund
Copy link
Collaborator

@Kneemund Kneemund commented Oct 31, 2024

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 Spellcheck #271.
  • Uses binary trees for caching results and only checks modified words.

TODO

  • Add per-document settings for enabling/disabling spellcheck and setting the language.
  • Add interface for selecting and applying suggestions/corrections, e.g. a popover menu in the sidebar.
  • Allow locking spellcheck behind a password (Spellcheck #271 (comment)).
  • Adapt Windows build workflow after deciding which languages to bundle.

Things to Discuss

  • Spellcheck correction picker.
  • Enchant apparently supports multi-dictionary spellchecking. We could also support that, the main problem is likely adapting the UI (GtkMultiSelection).
  • 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_locale and icu_experimental. They're used to convert the IETF region tags to display names. icu as a whole seems quite useful though (for formatting lists, etc.), so we could consider adding the entire meta-crate instead. Either way, the two crates have a lot of dependencies themselves. I couldn't find many alternatives though, apart from lcid.
  • 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 Kneemund changed the title feat: spellcheck for typewriter strokes feat: spellcheck for text strokes Nov 1, 2024
@Kneemund
Copy link
Collaborator Author

Kneemund commented Nov 2, 2024

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
Copy link
Collaborator Author

Kneemund commented Nov 3, 2024

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
Copy link
Collaborator Author

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:

spellcheck_corrections.mp4

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
Copy link
Collaborator Author

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

corrections.mp4

@Doublonmousse
Copy link
Collaborator

Doublonmousse commented Nov 27, 2024

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

@Kneemund
Copy link
Collaborator Author

Kneemund commented Nov 27, 2024

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).

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.

Spellcheck
2 participants