dfaure added inline comments.
INLINE COMMENTS
> colorscope.h:131
> +QObject *const m_parent;
> +Plasma::Theme::ColorGroup m_actualGroup;
>
valgrind says this member isn't initialized, please fix.
http://www.davidfaure.fr/2018/colorscope_valgrind_log.txt
REPOSITORY
R242 Plasma
This revision was automatically updated to reflect the committed changes.
Closed by commit R242:f8047e86b298: Reduce the amount of spurious property
changes on ColorScope (authored by apol).
REPOSITORY
R242 Plasma Framework (Library)
CHANGES SINCE LAST UPDATE
apol updated this revision to Diff 23233.
apol added a comment.
Fix david's comment
REPOSITORY
R242 Plasma Framework (Library)
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D8917?vs=23202=23233
BRANCH
master
REVISION DETAIL
https://phabricator.kde.org/D8917
AFFECTED FILES
mart added a comment.
ah, you are right, yes, it should do checkcologgroupchanged instead
REPOSITORY
R242 Plasma Framework (Library)
REVISION DETAIL
https://phabricator.kde.org/D8917
To: apol, #plasma, mart, davidedmundson
Cc: davidedmundson, plasma-devel, #frameworks, ZrenBot,
davidedmundson added inline comments.
INLINE COMMENTS
> mart wrote in colorscope.cpp:193
> this is when the item changes window and we're not sure we are still in the
> same color set, so i think is ok to keep this signal
yeah, my point was we may /also/ need the colorsChanged signal (via
mart accepted this revision.
mart added inline comments.
INLINE COMMENTS
> davidedmundson wrote in colorscope.cpp:193
> check here.
this is when the item changes window and we're not sure we are still in the
same color set, so i think is ok to keep this signal
REPOSITORY
R242 Plasma
davidedmundson added a comment.
description needs updating with the new benefits (saving lookups every time)
INLINE COMMENTS
> colorscope.cpp:193
> if (value.window) {
> emit colorGroupChanged();
> }
check here.
REPOSITORY
R242 Plasma Framework (Library)
apol updated this revision to Diff 23202.
apol added a comment.
Move to tracking parents instead of doing a look-up on every color get
REPOSITORY
R242 Plasma Framework (Library)
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D8917?vs=22664=23202
BRANCH
master
REVISION DETAIL
davidedmundson requested changes to this revision.
davidedmundson added a comment.
This revision now requires changes to proceed.
Concept is good.
INLINE COMMENTS
> colorscope.cpp:127
> }
> +m_lastGroup = m_group;
> return m_group;
It's weird to be caching in a public getter.
apol created this revision.
apol added reviewers: Plasma, mart.
Restricted Application added projects: Plasma, Frameworks.
Restricted Application added subscribers: Frameworks, plasma-devel.
REVISION SUMMARY
At the moment whenever something changed we were emitting colorGroupChanged
and then
10 matches
Mail list logo