----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124282/#review82252 -----------------------------------------------------------
some style nitpicks from my side, but it looks very good already - nice work! src/plugins/voikko/voikkoclient.cpp (line 28) <https://git.reviewboard.kde.org/r/124282/#comment56631> here and elsewhere: the Q_FUNC_INFO is not required src/plugins/voikko/voikkodict.cpp (line 38) <https://git.reviewboard.kde.org/r/124282/#comment56632> please wrap these in functions to remove static global initialization and delay it to the point where its actually required src/plugins/voikko/voikkodict.cpp (line 109) <https://git.reviewboard.kde.org/r/124282/#comment56633> join next line src/plugins/voikko/voikkodict.cpp (line 132) <https://git.reviewboard.kde.org/r/124282/#comment56634> while you properly delete it, I'd still urge you to cleanup the code by using `std::unique_ptr` if, in the future, you want to optimize your code, you should introduce a buffer (`std::vector<wchar_t>`) and reuse that whenever you need to convert a string to `wchar_t`. Now, you always allocate a new buffer, which is bad, performance wise src/plugins/voikko/voikkodict.cpp (line 152) <https://git.reviewboard.kde.org/r/124282/#comment56639> if (!voikkoSuggestions) { src/plugins/voikko/voikkodict.cpp (line 191) <https://git.reviewboard.kde.org/r/124282/#comment56635> return !m_handle; src/plugins/voikko/voikkodict.cpp (line 201) <https://git.reviewboard.kde.org/r/124282/#comment56636> rtrim src/plugins/voikko/voikkodict.cpp (line 204) <https://git.reviewboard.kde.org/r/124282/#comment56637> if (!userDictFile.open(...)) { src/plugins/voikko/voikkodict.cpp (line 219) <https://git.reviewboard.kde.org/r/124282/#comment56638> join next line - Milian Wolff On July 8, 2015, 5:10 p.m., Jesse Jaara wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/124282/ > ----------------------------------------------------------- > > (Updated July 8, 2015, 5:10 p.m.) > > > Review request for KDE Frameworks and Martin Tobias Holmedahl Sandsmark. > > > Repository: sonnet > > > Description > ------- > > # Implement Voikko based spellchecker for Sonnet > > ## Description > Implements a spell chekcing plugin based on libvoikko > <http://voikko.puimula.org/>. > Primarily for supporting highquality Finnishs spell checking, but HFST > trancuders > can be found several other languages too. > <http://sourceforge.net/projects/hfst/files/resources/spell-transducers/> > > > ## List of commits (oldest 1st) > --------------------------------------------------------------------------------------------------- > > Define QLoggingCategory for for voikko speller plugin > > * Declared SONNET_VOIKKO QLoggingCategory > > -------------------------------------------------------------------------------------------------- > > Implement Voikko based spellchecker (dictionary) > > * All Sonnet::SpellerPlugin functions are implemented. > * storeReplacement() and addToPersonal() use Json based storage. > * File location: > * UNIX & OSX: > QStandardPaths::GenericDataLocation/Sonnet/Voikko-user-dictionary.json > * Windows >= Vista: > QSP::GenericDataLocation/../Roaming/Sonnet/Voikko-user-dictionary.json > * XP: QSP::GenericDataLocation/../../Aplication > Data/Sonnet/Voikko-user-dictionary.json > * Format: > ```JSON > { "<languageId>": { > "PersonalWords": [ > "word" > ], > "Replacements": [ > {"bad": "eror", > "good": "error"} > ] > } > ``` > * Before use VoikkoDict based chekkers must be ensured to be with valid with > initFailed(). > As so the ctor is protected and only accessible from friens class > VoikkoClient, which > does this check before returning the speller. Using an invalid speller will > result in > null-pointer exceptions. > > -------------------------------------------------------------------------------------------------- > > Implement Sonnet::Client for Voikko speller > > * Reliability set to 50. > Voikko is primarily only used for Finnish at the moment, although > the HFST transducer-backend has added support for other languages > of varying quality. > As for Finnish (99% of use cases) the results are top quality. > > In any case the reliability should be higher than that of hunspell > and aspell to prevent them from kicking in for Finnish, as the > Finnish dictionarys for them are low-quality. > > * Name is "Voikko" > > -------------------------------------------------------------------------------------------------- > > Add in CMakeBits needed to compile Voikko speller. > > * Added FindVOIKKO module > > > Diffs > ----- > > src/plugins/CMakeLists.txt 3d24d61 > cmake/FindVOIKKO.cmake PRE-CREATION > src/plugins/voikko/CMakeLists.txt PRE-CREATION > src/plugins/voikko/voikkoclient.h PRE-CREATION > src/plugins/voikko/voikkoclient.cpp PRE-CREATION > src/plugins/voikko/voikkodebug.h PRE-CREATION > src/plugins/voikko/voikkodebug.cpp PRE-CREATION > src/plugins/voikko/voikkodict.h PRE-CREATION > src/plugins/voikko/voikkodict.cpp PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/124282/diff/ > > > Testing > ------- > > > Thanks, > > Jesse Jaara > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
