ervin requested changes to this revision.
ervin added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> componentchooser.cpp:80
> +
> + if (cfgType==QLatin1String("internal_email")) {
> + loadedConfigWidget = new CfgEmailClient(this);
I know it's older code but since you touched it, what about adding the spaces
around == ?
Likewise for the few ifs below.
> componentchooser.cpp:98
>
> -void ComponentChooser::slotServiceSelected(QListWidgetItem* it) {
> - if (!it) return;
> -
> - if (somethingChanged) {
> - if (KMessageBox::questionYesNo(this,i18n("<qt>You changed the
> default component of your choice, do want to save that change now
> ?</qt>"),QString(),KStandardGuiItem::save(),KStandardGuiItem::discard())==KMessageBox::Yes)
> save();
> - }
> - const QString &service = it->data(Qt::UserRole).toString();
> - KConfig cfg(service, KConfig::SimpleConfig);
> -
> -
> ComponentDescription->setText(cfg.group(QByteArray()).readEntry("Comment",i18n("No
> description available")));
> - ComponentDescription->setMinimumSize(ComponentDescription->sizeHint());
> -
> - configWidget = configWidgetMap.value(service);
> - if (configWidget) {
> - configContainer->setCurrentWidget(configWidget);
> - const auto plugin = dynamic_cast<CfgPlugin*>(configWidget);
> - headerGroupBox->setTitle(it->text());
> - plugin->load(&cfg);
> - emit defaulted(plugin->isDefaults());
> - }
> -
> - emitChanged(false);
> - latestEditedService = service;
> -}
> +void ComponentChooser::emitChanged(bool val) {
>
Curly brace on its own line.
> componentchooser.cpp:100
>
> + bool somethingChanged = val;
> + if (!somethingChanged) {
Does that make any sense to keep the val parameter on that method if you loop
through all the plugins because of defaulted anyway?
If it goes away the two loops could thus be merged.
> componentchooser.cpp:103
> + // check if another plugin has changed
> + for (auto it= configWidgetMap.constKeyValueBegin(); it !=
> configWidgetMap.constKeyValueEnd(); ++it) {
> + CfgPlugin *plugin = dynamic_cast<CfgPlugin *>( (*it).second );
Space before = missing
> componentchooser.cpp:104
> + for (auto it= configWidgetMap.constKeyValueBegin(); it !=
> configWidgetMap.constKeyValueEnd(); ++it) {
> + CfgPlugin *plugin = dynamic_cast<CfgPlugin *>( (*it).second );
> + somethingChanged |= plugin->hasChanged();
No need for the spaces within the parenthesis
> componentchooser.cpp:111
> + bool isDefault = true;
> + for (auto it= configWidgetMap.constKeyValueBegin(); it !=
> configWidgetMap.constKeyValueEnd(); ++it) {
> + CfgPlugin *plugin = dynamic_cast<CfgPlugin *>( (*it).second );
Ditto
> componentchooser.cpp:112
> + for (auto it= configWidgetMap.constKeyValueBegin(); it !=
> configWidgetMap.constKeyValueEnd(); ++it) {
> + CfgPlugin *plugin = dynamic_cast<CfgPlugin *>( (*it).second );
> + isDefault &= plugin->isDefaults();
Ditto
> componentchooser.cpp:127
> void ComponentChooser::load() {
> - if( configWidget )
> - {
> - CfgPlugin * plugin = dynamic_cast<CfgPlugin*>( configWidget );
> - if( plugin )
> - {
> - KConfig cfg(latestEditedService, KConfig::SimpleConfig);
> - plugin->load( &cfg );
> - }
> + for (auto it= configWidgetMap.constKeyValueBegin(); it !=
> configWidgetMap.constKeyValueEnd(); ++it) {
> +
Space before =
> componentchooser.cpp:132
> +
> + CfgPlugin * plugin = dynamic_cast<CfgPlugin*>( widget );
> + if (plugin) {
No space after *, no space within parenthesis
> componentchooser.cpp:141
> void ComponentChooser::save() {
> - if( configWidget )
> - {
> - CfgPlugin* plugin = dynamic_cast<CfgPlugin*>( configWidget );
> - if( plugin )
> - {
> - KConfig cfg(latestEditedService, KConfig::SimpleConfig);
> + for (auto it= configWidgetMap.constKeyValueBegin(); it !=
> configWidgetMap.constKeyValueEnd(); ++it){
> +
Ditto
> componentchooser.cpp:146
> +
> + CfgPlugin* plugin = dynamic_cast<CfgPlugin*>( widget );
> + if (plugin) {
Ditto
REPOSITORY
R119 Plasma Desktop
REVISION DETAIL
https://phabricator.kde.org/D26797
To: meven, #plasma, #vdg, ngraham, ervin
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, jraleigh, zachus,
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai,
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
