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

List:       kde-commits
Subject:    Re: KDE/kdenetwork/kopete/protocols/oscar/icq/ui (silent) [POSSIBLY
From:       Matt Rogers <mattr () kde ! org>
Date:       2008-09-08 3:02:01
Message-ID: 4CD35E36-C10D-42F8-B0B1-CFD4978F82D9 () kde ! org
[Download RAW message or body]


On Sep 7, 2008, at 9:19 AM, Nickolai Shaforostoff wrote:

> SVN commit 858171 by shaforo:
>
> beautify fallback encoding autosetting for icq protocol

Some thoughts...

>
> SVN_SILENT

Don't use SVN_SILENT for commits like these. This isn't really a small  
change that warrants the use of SVN_SILENT.

>
>
> M  +43 -8     icqeditaccountwidget.cpp   [POSSIBLY UNSAFE: system]
>
>
> --- trunk/KDE/kdenetwork/kopete/protocols/oscar/icq/ui/ 
> icqeditaccountwidget.cpp #858170:858171
> @@ -17,13 +17,14 @@
> #include "icqeditaccountwidget.h"
> #include "ui_icqeditaccountui.h"
>
> -#include <qlayout.h>
> -#include <qcheckbox.h>
> -#include <qlineedit.h>
> -#include <qspinbox.h>
> -#include <qpushbutton.h>
> -#include <qvalidator.h>
> +#include <QLayout>
> +#include <QCheckbox>
> +#include <QLineedit>
> +#include <QSpinBox>
> +#include <QPushButton>
> +#include <QValidator>
> #include <QLatin1String>
> +#include <QLocale>
>
> #include <kconfig.h>
> #include <kdebug.h>
> @@ -137,8 +138,42 @@
> 	else
> 	{
> 		int encodingId=4; //see icqprotocol.cpp for mappings
> -		if (KGlobal::locale()->language().startsWith("ru"))
> -			encodingId=2251;
> +		switch (QLocale::system().language())

Why are you using QLocale here? Kopete won't honor the KDE settings  
for languages now. This makes KDE_LANG useless, among other possible  
side effects.

>
> +		{
> +			case QLocale::Russian:
> +			case QLocale::Ukrainian:
> +			case QLocale::Byelorussian:
> +			case QLocale::Bulgarian:
> +				encodingId=2251;
> +				break;
> +			case QLocale::Hebrew:
> +				encodingId=2255;
> +				break;
> +			case QLocale::Turkish:
> +				encodingId=2254;
> +				break;
> +			case QLocale::Greek:
> +				encodingId=2253;
> +				break;
> +			case QLocale::Arabic:
> +				encodingId=2256;
> +				break;
> +			case QLocale::German:
> +			case QLocale::Italian:
> +			case QLocale::Spanish:
> +			case QLocale::Portuguese:
> +			case QLocale::French:
> +			case QLocale::Dutch:
> +			case QLocale::Danish:
> +			case QLocale::Swedish:
> +			case QLocale::Norwegian:
> +			case QLocale::Icelandic:
> +				encodingId=2252;
> +				break;
> +			default:
> +				encodingId=4;
> +		}
> +

Perhaps you could define constants and use those for all the magic  
numbers here. Perhaps the mappings in icqprotocol.h are useful for you  
and those could be used, which would be even better

> 		
> 		mProtocol->setComboFromTable( mAccountSettings->encodingCombo,
> 		                              mProtocol->encodings(),
> 		                              encodingId );


Thanks
--
Matt

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