-
Notifications
You must be signed in to change notification settings - Fork 570
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
Fails to build against Hunspell 1.5 #261
Comments
|
Thanks for the heads-up. I should be able to take a look at this the beginning of the week. |
|
Please post that bug at Hunspell. They have changed the return type of get_wordchars unnecessarily and without any deprecation. Everything else is just newly added deprecations. If they are going to break the interface they should have incremented the major number.
Kevin
…Sent from my iPad
On Nov 26, 2016, at 4:25 AM, Jan Beich ***@***.***> wrote:
Broken by ***@***.***, the rest uses compat.
src/Misc/SpellCheck.cpp:127:24: warning: 'spell' is deprecated [-Wdeprecated-declarations]
return m_hunspell->spell(m_codec->fromUnicode(Utility::getSpellingSafeText(word)).constDa...
^
/usr/local/include/hunspell/hunspell.hxx:130:20: note: 'spell' has been explicitly marked deprecated
here
H_DEPRECATED int spell(const char* word, int* info = NULL, char** root = NULL);
^
src/Misc/SpellCheck.cpp:138:29: warning: 'suggest' is deprecated [-Wdeprecated-declarations]
int count = m_hunspell->suggest(&suggestedWords, m_codec->fromUnicode(Utility::getSpellin...
^
/usr/local/include/hunspell/hunspell.hxx:140:20: note: 'suggest' has been explicitly marked
deprecated here
H_DEPRECATED int suggest(char*** slst, const char* word);
^
src/Misc/SpellCheck.cpp:144:17: warning: 'free_list' is deprecated [-Wdeprecated-declarations]
m_hunspell->free_list(&suggestedWords, count);
^
/usr/local/include/hunspell/hunspell.hxx:154:21: note: 'free_list' has been explicitly marked
deprecated here
H_DEPRECATED void free_list(char*** slst, int n);
^
src/Misc/SpellCheck.cpp:210:52: warning: 'get_dic_encoding' is deprecated [-Wdeprecated-declarations]
m_codec = QTextCodec::codecForName(m_hunspell->get_dic_encoding());
^
/usr/local/include/hunspell/hunspell.hxx:157:28: note: 'get_dic_encoding' has been explicitly marked
deprecated here
H_DEPRECATED const char* get_dic_encoding() const;
^
src/Misc/SpellCheck.cpp:217:28: error: no matching member function for call to 'toUnicode'
m_wordchars = m_codec->toUnicode(m_hunspell->get_wordchars());
~~~~~~~~~^~~~~~~~~
/usr/local/include/qt5/QtCore/qtextcodec.h:78:13: note: candidate function not viable: no known
conversion from 'const std::string' (aka 'const basic_string<char, char_traits<char>,
allocator<char> >') to 'const QByteArray' for 1st argument
QString toUnicode(const QByteArray&) const;
^
/usr/local/include/qt5/QtCore/qtextcodec.h:79:13: note: candidate function not viable: no known
conversion from 'const std::string' (aka 'const basic_string<char, char_traits<char>,
allocator<char> >') to 'const char *' for 1st argument
QString toUnicode(const char* chars) const;
^
/usr/local/include/qt5/QtCore/qtextcodec.h:102:13: note: candidate function not viable: requires at
least 2 arguments, but 1 was provided
QString toUnicode(const char *in, int length, ConverterState *state = Q_NULLPTR) const
^
4 warnings and 1 error generated.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
Looks like hunspell's "fix" is to remove the changelog claim that the api for 1.5 remains compatible with 1.4. The comment section for the issue @jbeich raised with Hunspell clearly indicates they're not going to fix it. This is going to cause problems when Sigil is built against systems that use hunspell v1.5 (think repo builds). |
|
The problem is in my early testing of hunspell a few months back, the newer C++ style interface and code was noticeably slower on older systems than the old c-like code especially when multiple threads are used. Given we do inline real-time spellchecking when editing in Sigil, I really want Hunspell to have a smallfootprint and be faster not slower. Perhaps they have fixed things in the last few months? If not, we are not moving to slower more bloated code just to make it more C++ like. Silly, really, MySpell and Hunspell its replacement both used the c-like code approach because it had a smaller footprint, could compile on just about any compiler and system, and was faster at that time then poor and inconsistent STL based approaches. |
|
I have no issue with continuing to bundle 1.4 with Sigil. The problem is that the Linux repo versions of Sigil will be built against the system version of hunspell and not the bundled version. So Sigil will need to be able to build (using the -DUSE_SYSTEM_LIBS option) on say Arch using hunspell 1.5 or Ubuntu/Debian using 1.4. Hunspell really screwed the pooch here (in my opinion) by introducing interface-breaking changes before v2.0. The question is: can Sigil be (easily) modified to work with either hunspell version when -DUSE_SYSTEM_LIBS is specified in the configuration? |
|
Yes, we can make Sigil easily convert from c++ std:: char and string types to Qt QString and QChar types just as we do with char * to Qt types now. I will look into it.
…Sent from my iPad
On Nov 26, 2016, at 7:55 PM, Doug Massay ***@***.***> wrote:
I have no issue with continuing to bundle 1.4 with Sigil. The problem is that the Linux repo versions of Sigil will be built against the system version of hunspell and not the bundled version. So Sigil will need to be able to build (using the -DUSE_SYSTEM_LIBS option) on say Arch using hunspell 1.5 or Ubuntu/Debian using 1.4.
Hunspell really screwed the pooch here (in my opinion) by introducing interface-breaking changes before v2.0.
The question is: can Sigil be (easily) modified to work with either hunspell version when -DUSE_SYSTEM_LIBS is specified in the configuration?
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.
|
|
If you get a chance, Kevin, take a look at the issue @jbeich raised with hunspell over this and see if you have an opinion on the either of the new comments there. |
|
There was another api breakage reported for hunspell 1.5. One of the maintainers there has indicated that v1.5 will be pulled. Looks like the plan is to release a v1.5.1 that keeps the interface from 1.4 intact. |
|
Good to know. That is what they should have done from the beginning. I will still look at how to interface to the new versions, but there changes will eventually break plugin spellchecking as it needs/uses a python ctypes interface to libhunspell not C++ calls. |
|
The issue with hunspell v1.5 seems to have been resolved upstream. Should I leave this issue open as a reminder that Sigil will need to be modifed to deal with hunspell v2.0 whenever that happens? |
|
No, we can open a new issue when 2.0 happens as we will need to see what ctypes like python interface is possible with the 2.0 pure C++ interface. If none, then we will stay with 1.5.x |
Broken by hunspell/hunspell@64f34ca52f01, the rest uses compat.
The text was updated successfully, but these errors were encountered: