Review Request: Make kcodecs framework build standalone

[David Faure]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/105860/#review16961
-----------------------------------------------------------


QObject::tr() sets a context of "QObject", which won't match. You need to use QCoreApplicaton::translate() with an empty context, everywhere. This is a bit surprising for Qt developers, but it's the only way to translate strings from Qt programs with gettext (which doesn't know these "C++ classname contexts", a Qt invention).



tier1/kcodecs/src/kcharsets.cpp
<http://git.reviewboard.kde.org/r/105860/#comment13331>

    I see, the approach works, but can surprise the reader: this array is not used, but is useful (to mark the strings up for translation). (Also, it duplicates strings from the other array, but it's generated so that's ok).
    Maybe a comment should be added on top of the array. Hmm, and I guess the compiler warns about the unused array, too...
    
    Another approach would be the one used by the Qt documentation for QT_TRANSLATE_NOOP3, i.e. back to a single array, but with the struct {source,comment} as item type, and for non-translated codecs (such as "ISO 8859-1"), just write
    { "ISO 8859-1", "" }.
    Ah I see, this loses the whole benefit of "a single char array with indices", i.e. it makes the use of the kdesdk script unnecessary. But well, if we're duplicating half the strings into char* anyway, we're not saving memory, on the contrary....
    I think I would personnally go for this - ditching the use of the script for this data, and just having one array, as in the Qt documentation.
    



tier1/kcodecs/src/kcharsets.cpp
<http://git.reviewboard.kde.org/r/105860/#comment13328>

    You wrote "characeter" instead of "character" everywhere :)
    



tier1/kcodecs/src/kcharsets.cpp
<http://git.reviewboard.kde.org/r/105860/#comment13329>

    trailing whitespace (coming from the script?). Please remove, to avoid unnecessary whitespace changes when an editor removes them later on.


- David Faure


On Aug. 5, 2012, 2:24 p.m., George Goldberg wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105860/
> -----------------------------------------------------------
> 
> (Updated Aug. 5, 2012, 2:24 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Description
> -------
> 
> Make it possible to build kcodecs independently of the whole kdelibs-frameworks tree.
> 
> 
> Diffs
> -----
> 
>   tier1/kcodecs/CMakeLists.txt 22463ca0345e6d50384c17bf5f43824e3445b55f 
>   tier1/kcodecs/src/kcharsets.cpp c64aa12361583cd9184bc39495c7149e6b0d7796 
>   tier1/kcodecs/src/kencodingprober.cpp 1dad3272986c2b0c693cd1bbf49fd40055300e39 
> 
> Diff: http://git.reviewboard.kde.org/r/105860/diff/
> 
> 
> Testing
> -------
> 
> Works for me, both standalone and as part of kdelibs
> 
> 
> Thanks,
> 
> George Goldberg
> 
>

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