D26797: KCM/Component Refactor UI to a single list of combobox

[Kevin Ottens]

ervin requested changes to this revision.
ervin added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> componentchooser.cpp:62
> +        // fill the form layout
> +        const auto name = cg.readEntry("Name",i18n("Unknown"));
> +        CfgPlugin *loadedConfigWidget = loadConfigWidget(cfg.group(QByteArray()).readEntry("configurationType"));

Missing space after comma

> componentchooser.cpp:106
> +    // check if another plugin has changed and default status
> +    for (CfgPlugin * plugin: configWidgetMap) {
> +        somethingChanged |= plugin->hasChanged();

No space after * and missing qAsConst

> componentchooser.cpp:137
>  void ComponentChooser::save() {
> -	if( configWidget )
> -	{
> -		CfgPlugin* plugin = dynamic_cast<CfgPlugin*>( configWidget );
> -		if( plugin )
> -		{
> -			KConfig cfg(latestEditedService, KConfig::SimpleConfig);
> +    for (auto it = configWidgetMap.constBegin(); it != configWidgetMap.constEnd(); ++it){
> +

Missing space before {

> componentchooser.cpp:142
> +
> +        CfgPlugin *plugin = dynamic_cast<CfgPlugin*>( widget );
> +        if (plugin) {

No space between parenthesis

> componentchooser.cpp:154
>  void ComponentChooser::restoreDefault() {
> -    if (configWidget)
> -    {
> -        dynamic_cast<CfgPlugin*>(configWidget)->defaults();
> -        emitChanged(true);
> +    for (CfgPlugin* plugin : configWidgetMap) {
> +        plugin->defaults();

Space before the * and not after. Also you could have used auto or auto * for all those loops (just that const auto & makes no sense in that context).

> ervin wrote in componentchooser.cpp:132
> No space after *, no space within parenthesis

No space within parenthesis

> broulik wrote in componentchooserbrowser.cpp:104
> As a future step I would like those default components not hardcoded in the code

Yes, it screams for GUI / config separation (in another commit)

REPOSITORY
  R119 Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D26797

To: meven, #plasma, #vdg, ngraham, ervin
Cc: filipf, broulik, 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20200128/7d469cbd/attachment-0001.html>