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

_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Reply via email to