Add experimental spellchecker #1424

fxha · 2019-10-01T15:55:11Z

Q	A
New feature?	yes
BC breaks?	yes
Fixed tickets	#168
License	MIT

Description

This PR should implement an experimental spell checker to correct misspelled words. An advanced checker like LanguageTool (grammar, spelling and punctuation checker) would have to be integrated with Muya.

Experimental spell checker for testing purpose only
Requires Node 12
Please run yarn add file:packages/electron-spellchecker between commits to update the package without increasing version number
Use dev tools for debug and trace information

Chromium's version of Hunspell is used on Linux and Windows and on macOS the OS spell checker is used by default. Hunspell can be used on macOS but is completely untested.

NOTE: node-spellcheker uses Chromium's version of Hunspell that is licensed under GPL, LGPL or MPL (by choice). Chosen LGPL requires us to "dynamic link" the native library, so the user can replace the library.

Checklist:

node-spellcheker is dynamic linked
node-spellcheker can be replaced by the user on disk and is not bundled inside .asar
- Don't bundle vendor folder with GPL/LGPL/MPL code
Source code provided online
License attribution required for Hunspell

Todo:

-

Testing:

Code review of all changes
Set Invalid Test Value as language via settings and test spell checker behavior. Is there a warning that the dict was not found? Can you still use the spell checker? Are Spelling... -> Debug information right? @Jocs
Test spell checking on macOS - no testing done yet! @Jocs
- Test settings on macOS
- Test both Hunspell and OS spell checker
Test spell checking
- Linux
- macOS
- Windows 10

Feature:

Option to not underline spelling mistakes but allow to correct spelling mistakes via right click menu.

Improvements:

Enable language detection in another thread using node Web Workers for Hunspell (and Windows API). Until then we disabled the feature for Hunspell (and Windows API). This is currently blocked due to Electron #18540 upstream bug.
- Force language detection when loading a new tab with markdown content when using Hunspell. Otherwise language is only detected when the user types words.
Use Compact Language Detector v3 (wasm, neural network) instead v2 (node module, Naive Bayes classifier)?
Check for spelling only when the user stopped typing? Currently not possible due to Electron API?
Text editing becomes laggy with a larger document due to Rendering improvements needed #1289 and whole content is spell checked.

node-spellchecker improvements/changes:

Allow to change spell check provider at runtime (rebase from atom upstream).
Enable Windows native spell checking API for wider language support on Windows 10?
Run node-spellchecker asynchronous in threads: (rebase from atom upstream)
- Enable multithreading in Windows spell checker.
- Spawn cluster of Hunspell worker threads.
Update Hunspell library from Chromium upstream becasue the used version is 6 years old.

Known issues:

Loading Hunspell word suggestions takes a little longer and doesn't feel instant when opening right click menu (Hunspell upstream issue?).
Text editing becomes laggy with a larger document due to Rendering improvements needed #1289 and whole content is spell checked.

General settings:

Hunspell only settings:

fxha · 2019-10-09T18:39:39Z

@Jocs Could you please test the PR on macOS? Above are some areas to test for and please run yarn add file:packages/electron-spellchecker after each git pull to update the dependency. The code need still some ❤️ and testing and the improvements are planned in other PRs and/or require to fork node-spellcheker.

Jocs · 2019-10-10T13:12:05Z

Could you please test the PR on macOS? Above are some areas to test for and please run yarn add file:packages/electron-spellchecker after each git pull to update the dependency. The code need still some ❤️ and testing and the improvements are planned in other PRs and/or require to fork node-spellcheker.

Great work, I'll test it this evening or tomorrow.

src/renderer/contextMenu/editor/index.js

Jocs · 2019-10-10T16:01:49Z

src/muya/lib/index.js

+   * @param {string} replacement The replacement.
+   * @param {boolean} setCursor Shoud we update the editor cursor?
+   */
+  replaceWordInline (line, wordCursor, replacement, setCursor = false) {


I prefer put this method on ContentState.prototype, for example create a file name spellCheckerCtrl.js.

ContentState.prototype. replaceWordInline = function () {}

That's actually a good idea but for a high level API or Muya API to transform lines. Do you have any ideas where to add the API interface?

The API interface need to put on the muya instance,

Muya { replaceWordInline (...args) {return this.contentState. replaceWordInline(...args)} }

is it ok?

How about to add a core contentState class/file with all basic editor operations like replace, add line, remove line, etc? So we could easily integrate the operation with the interface.

How about to add a core contentState class/file with all basic editor operations like replace, add line, remove line, etc? So we could easily integrate the operation with the interface.

Yeh, we need to abstract some operations, and then add them when optimizing muya.

Jocs · 2019-10-10T16:03:52Z

@fxha I'll take a full detail code review tomorrow.

Jocs · 2019-10-10T16:04:39Z

BTW, I love this feature, because I always make typo.

Jocs · 2019-10-12T07:19:09Z

Use experience as new user

Spell icon is the same as General icon, (I'll replace it other PR after this PR merged)
Also in the preferences window setting, whether Hunspell or OS spell checker, I don't know the switcher is on respect Hunspell or OS spell checker?
There are two English menu item in the drop down menu options. ~~where is the table (update and delete), I see them in previous review?~~ my fault, it need to turn on Hunspell ?
Can we have ignore option in editor context menu, only ignore them in current opened markdown file?

Development question?

Why we add electron-spellchecker to our repo?

Jocs · 2019-10-12T07:22:50Z

Open questions:

HTML blocks when editing in preview mode are not highlighted but can be corrected via context menu. Should we disable this "feature" and disallow corrections?

I prefer disable this feature, because we don't have to write whole word in html block.

Should we also (try) to integrate CodeMirror spell checking?

Does CodeMirror has spell checking?

Jocs · 2019-10-12T07:27:52Z

When I open a markdown file by marktext, this error dialog popup

My Spelling Setting as bellow:

Jocs · 2019-10-12T07:34:15Z

Set Invalid Test Value as language via settings and test spell checker behavior. Is there a warning that the dict was not found? Can you still use the spell checker? Are Spelling... -> Debug information right?

Warning dialog shown as bellow

the spell checker is not work now when I set to Invalid Test Value, and the editor context menu has no spell menu item, so I can not debug.

Jocs · 2019-10-12T07:42:53Z

When Whether Hunspell or the OS is on, the spell checker is not work on macOS. but it works when it's off.

src/index.ejs

src/main/windows/editor.js

src/muya/lib/index.js

src/renderer/contextMenu/editor/index.js

src/renderer/contextMenu/editor/spellcheck.js

src/renderer/prefComponents/sideBar/config.js

fxha · 2019-10-12T16:51:39Z

Also in the preferences window setting, whether Hunspell or OS spell checker, I don't know the switcher is on respect Hunspell or OS spell checker?

👍

There are two English menu item in the drop down menu options

When you use the OS spell checker, right? That means you OS support two English dictionaries that we cannot assign to a nation (e.g. en-US, en-GB, en-AU, ...). I'll add the 4-letter ISO code behind the language when we cannot map it exactly.

Can we have ignore option in editor context menu, only ignore them in current opened markdown file?

👍

Why we add electron-spellchecker to our repo?

That's only for development for this PR because I changed/modified a lot of the original code. I'll move our version of electron-spellchecker to another repository and npm when all changes are done.

Should we also (try) to integrate CodeMirror spell checking?

Does CodeMirror has spell checking?

I just did a quick reseach but we could try to integrate it to our spellcheck wrapper. It should be possible, maybe improvement for later.

fxha · 2019-10-12T17:26:25Z

@Jocs Could you please test again. If the exception still occurs, please post the stack trace from dev tools or log file. Could you also verify whether a dictionaries/en-US.bdic file is located in your application data directory in development mode (marktext-dev) and production. If not please send me your process.resourcesPath result for both dev and production.

Jocs · 2019-10-12T18:43:02Z

That's only for development for this PR because I changed/modified a lot of the original code. I'll move our version of electron-spellchecker to another repository and npm when all changes are done.

👍

I just did a quick reseach but we could try to integrate it to our spellcheck wrapper. It should be possible, maybe improvement for later.

👍

Jocs · 2019-10-13T14:07:51Z

Could you please test again. If the exception still occurs, please post the stack trace from dev tools or log file.

I can not reproduce it now.

Could you also verify whether a dictionaries/en-US.bdic file is located in your application data directory in development mode (marktext-dev) and production.

Yes, there is a dictionaries/en-US.bdic file is located in your application data directory.

fxha · 2019-10-14T21:07:34Z

@Jocs I pushed electron-spellchecker as @hfelix/electron-spellchecker and removed all debug stuff. Please build and test again. BTW singleRender(line) doesn't work for word replacement because an exception is thrown.

fxha added 🐜 in progress This issue is in progress todo 🗒️ labels Oct 1, 2019

fxha force-pushed the experimental/spellchecker branch 5 times, most recently from 11d8380 to 1f43da4 Compare October 8, 2019 17:37

fxha removed the todo 🗒️ label Oct 9, 2019

Jocs reviewed Oct 10, 2019

View reviewed changes

fxha added 🔍 need review This PR need review and removed 🐜 in progress This issue is in progress labels Oct 11, 2019

fxha requested a review from Jocs October 11, 2019 18:26

Jocs reviewed Oct 12, 2019

View reviewed changes

fxha force-pushed the experimental/spellchecker branch from 1aee166 to 733ec71 Compare October 14, 2019 21:04

fxha changed the title ~~[WIP] Experimental spellchecker for testing purpose~~ Add experimental spellchecker Oct 14, 2019

fxha requested a review from Jocs October 14, 2019 21:05

fxha added 26 commits October 15, 2019 20:54

Fix Hunspell switchLanguage bug and improvements

5b677e1

Fix attach to editor when enabling spell check again

17938be

Add Hunspell license

baac147

Copy default Huspell dictionary on first start

cc19df1

Fix full language name

e0f18f0

Some code improvements

c5d4e21

Allow to add words to user dict and bug fixes

e70a8b9

Allow to change Muya's spellcheck container attribute

4f0b804

feat: Don't underline misspelled words

9f60101

Allow to set Hunspell on macOS

33ad700

Fix spellchecker changed value

1857a4a

Refactor switchLanguage, init and enableSpellchecker

382bc57

Refactor and some fixes

f850a4d

Code improvements

5d7a21c

electron-spellchecker cleanup and optimization

e793b98

Disable automatic language detection for Hunspell

4ca6f2d

Fix init on macOS and update JSDoc

0971d96

Fix macOS issues and some improvements

d39b5d0

Load single settings value only

4d33a64

Fix rebase

4011e6b

Remove debug code

b5650d3

Move electron-spellchecker to scoped npm repo

04a01be

Fix dictionary of ignored words on macOS

065d5d7

Move replaceWordInline to core API

49d5d56

Remove comment block

2238e1c

Fix upstream lint error

e0f172c

fxha force-pushed the experimental/spellchecker branch from 0e579b7 to e0f172c Compare October 15, 2019 18:55

Jocs approved these changes Oct 17, 2019

View reviewed changes

Jocs merged commit 603ed04 into develop Oct 17, 2019

Jocs deleted the experimental/spellchecker branch October 26, 2019 16:39

Uh oh!

Add experimental spellchecker #1424

Add experimental spellchecker #1424

Uh oh!

Conversation

fxha commented Oct 1, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

fxha commented Oct 9, 2019

Uh oh!

Jocs commented Oct 10, 2019

Uh oh!

Uh oh!

Uh oh!

Jocs Oct 10, 2019

Choose a reason for hiding this comment

Uh oh!

fxha Oct 10, 2019

Choose a reason for hiding this comment

Uh oh!

Jocs Oct 11, 2019

Choose a reason for hiding this comment

Uh oh!

fxha Oct 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jocs Oct 13, 2019

Choose a reason for hiding this comment

Uh oh!

Jocs commented Oct 10, 2019

Uh oh!

Jocs commented Oct 10, 2019

Uh oh!

Jocs commented Oct 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Jocs commented Oct 12, 2019

Uh oh!

Jocs commented Oct 12, 2019

Uh oh!

Jocs commented Oct 12, 2019

Uh oh!

Jocs commented Oct 12, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fxha commented Oct 12, 2019

Uh oh!

fxha commented Oct 12, 2019

Uh oh!

Jocs commented Oct 12, 2019

Uh oh!

Jocs commented Oct 13, 2019

Uh oh!

fxha commented Oct 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

fxha commented Oct 1, 2019 •

edited

Loading

fxha Oct 12, 2019 •

edited

Loading

Jocs commented Oct 12, 2019 •

edited

Loading

fxha commented Oct 14, 2019 •

edited

Loading