D21655: KSelectionOwner usage in ApplicationX11 and Compositor classes

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

INLINE COMMENTS

> composite.cpp:71-89
> +class CompositorSelectionOwner : public KSelectionOwner
>  {
> -    connect (this, SIGNAL(lostOwnership()), SLOT(looseOwnership()));
> -}
> -
> -void CompositorSelectionOwner::looseOwnership()
> -{
> -    owning = false;
> -}
> +public:
> +    CompositorSelectionOwner(const char *selection)
> +        : KSelectionOwner(selection, connection(), rootWindow())
> +        , m_owning(false)
> +    {

It's perfectly fine to keep declaration of CompositorSelectionOwner class in the header file. It's also worth to point out that KSelectionOwner is a QObject, which means CompositorSelectionOwner needs to have a Q_OBJECT macro.

> main_x11.h:27-40
> -class KWinSelectionOwner
> -    : public KSelectionOwner
> -{
> -    Q_OBJECT
> -public:
> -    explicit KWinSelectionOwner(int screen);
> -protected:

I don't see why we need to move this class to the cpp file. It can stay here.

REPOSITORY
  R108 KWin

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

To: romangg, #kwin, zzag
Cc: sbergeron, zzag, kwin, LeGast00n, jraleigh, fbampaloukas, GB_2, mkulinski, ragreen, jackyalcine, Pitel, iodelay, bwowk, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart