Compiz

Merge lp://qastaging/~ksamak/compiz/add_startup_option_to_negative into lp://qastaging/compiz/0.9.13

add_startup_option_to_negative
Merge into 0.9.13

Proposed by ksamak on 2017-01-26

Status:	Merged
Approved by:	Marco Trevisan (Treviño) on 2017-11-15
Approved revision:	4108
Merged at revision:	4136
Proposed branch:	lp://qastaging/~ksamak/compiz/add_startup_option_to_negative
Merge into:	lp://qastaging/compiz/0.9.13
Diff against target:	30 lines (+13/-1) 1 file modified plugins/neg/src/neg.cpp (+13/-1)
To merge this branch:	bzr merge lp://qastaging/~ksamak/compiz/add_startup_option_to_negative
Related bugs:	Link a bug report

Reviewer	Date Requested	Status
Marco Trevisan (Treviño)	2017-01-26	Approve on 2017-11-15
ksamak (community)		Approve on 2017-08-24
Andrea Azzarone		Needs Fixing on 2017-03-29
Sam Spilsbury	2017-01-26	Pending
Review via email: mp+315657@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2017-01-20.

Commit message

neg: added hook to react to change in ccsm, for autostart option

Description of the change

neg: added hook to react to change in ccsm, for autostart option

Revision history for this message

Sam Spilsbury (smspillaz) on 2017-01-26: Posted in a previous version of this proposal

review: Needs Fixing

Revision history for this message

Sam Spilsbury (smspillaz) wrote on 2017-01-26: Posted in a previous version of this proposal

> optionSetActivateAtStartupNotify (boost::bind (&NegScreen::optionChanged, this,
> _1, _2));

This is also incorrect - it will cause all windows to be toggled whenever the option is changed, not just on startup.

Revision history for this message

ksamak (ksamak) wrote on 2017-01-26: Posted in a previous version of this proposal

> > optionSetActivateAtStartupNotify (boost::bind (&NegScreen::optionChanged,
> this,
> > _1, _2));
>
> This is also incorrect - it will cause all windows to be toggled whenever the
> option is changed, not just on startup.

Yes, this is a suggestion made by 3v1n0, that when the user switches this option in ccsm, he sees a immediate reaction. This ensures the user that changes have been taken into account
This switches all windows immediately indeed. Though i can think of no case where it is irrelevant.

Revision history for this message

Marco Trevisan (Treviño) (3v1n0) wrote on 2017-03-08:

You need to fix indentation...

Revision history for this message

Andrea Azzarone (azzar1) wrote on 2017-03-29:

If this is the wanted behavior change the name of the option otherwise this does not look correct to me.

review: Needs Fixing

lp://qastaging/~ksamak/compiz/add_startup_option_to_negative updated on 2017-08-24

4108. By ksamak <ksamak@ksalaptop> on 2017-08-24: fixed indent

Revision history for this message

ksamak (ksamak) wrote on 2017-08-24:

> If this is the wanted behavior change the name of the option otherwise this
> does not look correct to me.

I now fixed the indent.

The original wanted behaviour is the start-up mechanism. it has been requested by 3v1n0 that the option has an effect when ticked, to show that it's been taken into account.

If first did with a timer, but i changed it to this "workaround", and it does fulfil both these requirements.

The only interest of this is the activation of negative windows at start up, so i do not see a semantic problem with the naming, but maybe I'm missing something.

review: Approve

Revision history for this message

Marco Trevisan (Treviño) (3v1n0) wrote on 2017-11-15:

Let's go with this...

review: Approve