search

electron-userland / electron-spellchecker

Implement spellchecking, correctly
MIT License
238 stars 83 forks source link

@felixrieseberg/spellchecker CheckSpelling not working #147

Open jeremyspiegel opened 5 years ago

jeremyspiegel commented 5 years ago

This package (electron-spellchecker) depends on @felixrieseberg/spellchecker. That npm package points to https://github.com/atom/node-spellchecker as its git repo, but it seems that @felixrieseberg/spellchecker is diverged from that code (@felixrieseberg/spellchecker is at version 4.0.8 but atom/node-spellchecker is at 3.5.3). I can't find a public place for the source for @felixrieseberg/spellchecker.

In the switch from using @nornagon/spellchecker to @felixrieseberg/spellchecker for electron-spellchecker, the functionality of checkSpelling seems to have been broken. In main.cc inside @felixrieseberg/spellchecker, I found the following commented out code in Spellchecker::CheckSpelling:

    std::vector<uint16_t> text(string->Length() + 1);

    // Why did we do that?
    // string->Write(reinterpret_cast<uint16_t *>(text.data()));

    Spellchecker* that = Nan::ObjectWrap::Unwrap<Spellchecker>(info.Holder());
    std::vector<MisspelledRange> misspelled_ranges = that->impl->CheckSpelling(text.data(), text.size());

This makes it so that we're not actually passing the input down into the hunspell library. The string->Write(reinterpret_cast<uint16_t *>(text.data())); wasn't commented out in the 4.0.7 version in @nornagon/spellchecker.

@felixrieseberg, why was this code commented out, and where is the source maintained? Thanks!

felixrieseberg commented 5 years ago

👋 Hey! This module is a bit in a wild state - it was born out of multiple teams collaborating on spellchecking in Electron apps, but we've all taken slightly different paths without a strong interest to combine them again. @nornagon and I are colleages and trade back and forth with fixing this damn thing. My source is here, which to be fair, is virtually impossible to find.

That being said, thanks a ton for taking a look - I'll spend some more time on this today and tomorrow, hopefully hunting down the mistake!

jeremyspiegel commented 5 years ago

👋Hi! Thanks for taking a look! Sorry I missed that branch in your fork before.