-
Notifications
You must be signed in to change notification settings - Fork 108
[email protected]: compilation failed #127
Comments
Like #112 related to Electron 5, we mentioned in #112 (comment) that we're behind on Electron versions (the latest Atom beta 1.42.0-beta0 is on Electron 4.2.7) so this isn't something we'll likely get to in the near term but may be open to reviewing a well-written pull request in the future. |
I'm kindly pinging @goddessfreya who has handled seems to be a similar issue before. I also hope some big players / for-profit companies that use this module could jump into the issue. |
@vladimiry Not the prettiest fixes, but here they are: #128 Now EDIT: Apparently I wrote the patch accidentally against an older version of this package that didn't have |
@goddessfreya your changes made the module compilable and working with Electron v7 (which comes with node v12.8.1). Thanks a lot, especially given the fact that you don't really use anything node related and this module in particular. The issue remains open until the fix lands onto the master branch. |
* TODO (switch back to upstream "node-spellchecker") atom/node-spellchecker#127 * TODO (blocker) electron/electron#20700 * TODO (blocker @electron v8) electron/electron#21154
@nathansobo any chance that new minor version can be released soon with provided PR? It's blocking many of us to update on electron 7 and test electron beta 8. Thanks in advance. |
@cumajkeee - please remember that Electron 8 introduces a native spellchecker support so such libraries won't be necessary :) But at this moment spellchecker works up to beta.4 - later versions have some problems: electron/electron#21798 |
@dziudek thanks for the note, wasn't aware of those changes. But still would like to integrate v7. |
@cumajkeee Note that Electron 8 brings native spellchecker support but doesn't provide a custom API which means a custom editor implementation can't be used with the new Electron API. |
@dziudek, in my opinion, at least a few major Electron releases should be published until the chromium-builtin/native spellchecker gets stabilized in Electron. In my experience, spellchecking is a sensitive topic if it comes to multi-OS and different Linux package formats support (Snap packages, for example, quite often require special treatment). There also might be a need for flexibilities mentioned by @astoilkov in the previous message. Besides that, new Electron versions, unfortunately, quite often come with broken working before things, sometimes broken severely, but the @cumajkeee just fork the https://github.com/goddessfreya/node-spellchecker (see #130) to your GitHub profile and then reference it in packages.json like instead of using the upstream/NPM module. But in this case, you better remove the prebuilds and force rebuilding from sources. PS changed @nathansobo => @cumajkeee |
Are there any updates with this? We currently have to cherry-pick a specific commit of node-spellchecker to build signal-desktop for Arch Linux. :) |
@kpcyrd, working version available in #130. Btw there were recent commits https://github.com/atom/node-spellchecker/commits/master so maybe it's already working. I still use the @goddessfreya fork. |
The text was updated successfully, but these errors were encountered: