D26100: [KCM/Component] Convert to KConfigXT browser cfg and make default and reinit buttons work properly

Kevin Ottens noreply at phabricator.kde.org
Mon Dec 23 12:59:49 GMT 2019


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

INLINE COMMENTS

> componentchooser.cpp:207
> +
> +    CfgPlugin * plugin = dynamic_cast<CfgPlugin*>( configWidget );
> +    emit defaulted(plugin->isDefaults());

No spaces between the parenthesis, no space after *

> componentchooser.h:42
>  	virtual void defaults()=0;
> +    virtual bool isDefaults() const {
> +        return false;

Wrong indentation for that file which happens to be non standard

Also it's probably best to keep it pure-virtual to avoid the bastardization of something which looks like an interface into an abstract class.

> componentchooser.h:64
>  	void changed(bool);
> +    void defaulted(bool);
>  };

ditto

> componentchooser.h:94
>  	void changed(bool);
> +    void defaulted(bool);
>  

ditto

> componentchooserbrowser.cpp:112
> +                                      " ('x-scheme-handler/http' in ServiceTypes or 'x-scheme-handler/https' in ServiceTypes)");
> +    const auto &browsers = KServiceTypeTrader::self()->query(QStringLiteral("Application"), constraint);
>      for (const auto &service : browsers) {

Ref on a temporary sounds fishy to me.

> componentchooserbrowser.h:31
>  	void defaults() override;
> +    bool isDefaults() const override;
>  

Wrong indentation

REPOSITORY
  R119 Plasma Desktop

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

To: meven, crossi, #plasma, ngraham, ervin
Cc: plasma-devel, 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/20191223/be2ccd9f/attachment.html>


More information about the Plasma-devel mailing list