Implement color scheme selection from these installed in the system using KColorSchemeManager
ClosedPublic

Authored by apol on Jul 19 2016, 12:17 PM.

Details

Reviewers
zhigalin
Group Reviewers
KDevelop
Summary

Makes it possible for KDevelop to provide UI to choose a color scheme.

Test Plan

Tested locally

Diff Detail

Repository
R33 KDevPlatform
Branch
5.0
Lint
No Linters Available
Unit
No Unit Test Coverage
apol updated this revision to Diff 5303.Jul 19 2016, 12:17 PM
apol retitled this revision from to Implement color scheme selection from these installed in the system using KColorSchemeManager.
apol updated this object.
apol edited the test plan for this revision. (Show Details)
apol added reviewers: KDevelop, zhigalin.
Restricted Application added a subscriber: kdevelop-devel. · View Herald TranscriptJul 19 2016, 12:17 PM
apol updated this revision to Diff 5304.Jul 19 2016, 12:18 PM

Remove unrelated change

apol updated this revision to Diff 5305.Jul 19 2016, 12:20 PM

Don't install private API

zhigalin added inline comments.Jul 20 2016, 10:26 PM
shell/colorschemechooser.h
34

What is this?

63

null pointer is required?

66

const?

67

Why const & here?

mwolff added a subscriber: mwolff.Jul 20 2016, 10:54 PM
mwolff added inline comments.
shell/colorschemechooser.h
34

It is a so called forward declaration. We don't actually need to know the ins and outs of how this class looks like, just that it exists somewhere.

63

it's good practice to make sure the value is properly initialized. otherwise, it will contain random uninitialized memory garbage

66

yes, it's a getter and does not mutate the observable state of this object. thus it should be const

67

that is following the Qt coding guidelines, which says that you take nearly all (except small) values by const&. Not doing so would incur a copy operation, which may be expensive.

zhigalin added inline comments.Jul 20 2016, 11:04 PM
shell/colorschemechooser.h
34

Why this instead of

#include <KActionCollection>

??

67

Understood, thanks.

kfunk added a subscriber: kfunk.Jul 21 2016, 5:13 AM
kfunk added inline comments.
shell/colorschemechooser.h
34
zhigalin edited edge metadata.Jul 22 2016, 1:59 PM

See №127979
It should be OK now

zhigalin accepted this revision.Jul 29 2016, 8:54 AM
zhigalin edited edge metadata.
In D2220#42987, @kfunk wrote:

This has been pushed, right? (cf. https://git.reviewboard.kde.org/r/127979/)

Yes, it has.

This revision is now accepted and ready to land.Jul 29 2016, 8:54 AM
apol closed this revision.Aug 24 2016, 12:35 AM
kfunk removed a subscriber: kfunk.Mar 21 2017, 9:53 AM