----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124282/#review82192 -----------------------------------------------------------
src/plugins/voikko/voikkoclient.cpp (line 28) <https://git.reviewboard.kde.org/r/124282/#comment56578> here you can just use the Q_FUNC_INFO macro to get the same information (the function signature). src/plugins/voikko/voikkoclient.cpp (line 30) <https://git.reviewboard.kde.org/r/124282/#comment56579> plural is dictionaries src/plugins/voikko/voikkoclient.cpp (line 32) <https://git.reviewboard.kde.org/r/124282/#comment56580> I prefer to error out early («if (!dictionaries) return;»), less indentation and state to remember. src/plugins/voikko/voikkodict.h (line 64) <https://git.reviewboard.kde.org/r/124282/#comment56585> prefix all private members with m_ src/plugins/voikko/voikkodict.cpp (line 47) <https://git.reviewboard.kde.org/r/124282/#comment56581> ALL_UPPERCASE usually means defines (because they have slightly different behavior from other variables). src/plugins/voikko/voikkodict.cpp (line 97) <https://git.reviewboard.kde.org/r/124282/#comment56582> always use braces {} src/plugins/voikko/voikkodict.cpp (line 108) <https://git.reviewboard.kde.org/r/124282/#comment56584> just return true here, and you don't need the else block. src/plugins/voikko/voikkodict.cpp (line 125) <https://git.reviewboard.kde.org/r/124282/#comment56586> if (replacements.contains(word)) { suggestions.append(word); } is much more readable src/plugins/voikko/voikkodict.cpp (line 130) <https://git.reviewboard.kde.org/r/124282/#comment56588> can't you free the string again here? then you can do an early return if wcharSuggestions is empty. (I'd also prefer avoiding hungarian notiation -- typeVariablename.) src/plugins/voikko/voikkodict.cpp (line 140) <https://git.reviewboard.kde.org/r/124282/#comment56587> isn't this the wrong delete? It is declared as a pointer, not an array. src/plugins/voikko/voikkodict.cpp (line 172) <https://git.reviewboard.kde.org/r/124282/#comment56590> This is not very elegant. A better solution might be to split this up into two functions (fetchPersonal() that returns languageNode, and storePersonal() that takes a languageNode()), maybe? src/plugins/voikko/voikkodict.cpp (line 180) <https://git.reviewboard.kde.org/r/124282/#comment56589> error out early instead. also, QIODevice::Truncate? That will delete everything already in the file. src/plugins/voikko/voikkodict.cpp (line 236) <https://git.reviewboard.kde.org/r/124282/#comment56591> error out early - Martin Tobias Holmedahl Sandsmark On July 7, 2015, 2:44 p.m., Jesse Jaara wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/124282/ > ----------------------------------------------------------- > > (Updated July 7, 2015, 2:44 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 > ----- > > cmake/FindVOIKKO.cmake PRE-CREATION > src/plugins/CMakeLists.txt 3d24d61 > 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
