Review Request 124282: Implement Voikko based spellchecker for Sonnet

[Milian Wolff]

-----------------------------------------------------------
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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150709/dfaf65fa/attachment-0001.html>