[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: [PATCH] RFC: Changing the language of individual KDE programs
From:       Krzysztof Lichota <krzysiek () lichota ! net>
Date:       2007-03-29 8:21:32
Message-ID: 460B770C.8000009 () lichota ! net
[Download RAW message or body]


David Faure napisał(a):
> On Wednesday 28 March 2007, Krzysztof Lichota wrote:
>> +#include <klanguagebutton.h>
>> +#include <kpushbutton.h>
>> +
>> +class QGridLayout;
>> +class KPushButtonWithData;
> 
> All this can be removed now.

Right :)

>> +        SwitchApplicationLanguage, ///< @since 3.5.7
> Remove trailing comma.
>> +      menuSwitchLanguage = 5,
> Same here ;)

I prefer to put commas at last element, so that if another value is
added, the diff does not contain spurious information that previous line
is modified. It is useful for blame^Wline annotation in CVS ;)

>>  +    if (signalSender->inherits("KPushButton") == false)
> Inherits is fragile, it breaks when classes get renamed (as happened with Qt4).
> Please do
>   const KPushButton *removeButton = ::qt_cast<const KPushButton*>(signalSender);
>   if (!removeButton) {
>     kdError...
>     return;
>   }

I knew there must be such cast somewhere... Unfortunately
techbase.kde.org mentions only qobject_cast :(

> (Might need a const_cast here if find() doesn't work with the const pointer, not sure).

Yes, it is needed, I had it a few lines below :)

>> +        KConfigGroupSaver saver(config, "Locale");
> KConfigGroup was already in 3.x, no? Save time on the kde4 porting by doing
> KConfigGroup group(config, "Locale");
> group.writeEntry("Language", languageString);
> config->sync();

Done :)

> With the above, the patch looks fine to me, please commit.

Thanks for review and comments :) Committed :)

	Krzysztof Lichota




["signature.asc" (application/pgp-signature)]

[prev in list] [next in list] [prev in thread] [next in thread]