Add kcolorschemechooser support and related setting menu entry
ClosedPublic

Authored by tfjellstrom on Mar 29 2018, 3:46 PM.

Details

Summary

This patch takes code from kdevelop that add a settings menu entry to allow color scheme changes per instance of kate.

Diff Detail

Repository
R40 Kate
Lint
Lint Skipped
Unit
Unit Tests Skipped
tfjellstrom requested review of this revision.Mar 29 2018, 3:46 PM
tfjellstrom created this revision.
tfjellstrom added a reviewer: Kate.
mwolff added a subscriber: mwolff.Apr 3 2018, 7:48 PM

instead of duplicating it further, could we move the code into ktexteditor and make it reusable from there?

instead of duplicating it further, could we move the code into ktexteditor and make it reusable from there?

Yep, +1 for that.

But actually, to get to the basic problem: The issue is that KTextEditor configuration is global, or?

ngraham added a subscriber: ngraham.Apr 8 2018, 2:36 PM

@cullmann This patch is unrelated to KTextEditor settings itself. It only adds an action that allows to change the KDE color scheme per application. The name "KateColorScheme..." is misleading here, since it has nothing to do with Kate color schemes...

Indeed, maybe this class should be part of the KConfigWidgets framework? See https://api.kde.org/frameworks/kconfigwidgets/html/annotated.html for current list of classes - it already contains the color scheme stuff and is a tier3 framework (so many dependencies allowed).

@zhigalin Any comments from your side?

Ping. Any updates on this matter?

Restricted Application added a subscriber: kwrite-devel. · View Herald TranscriptMay 27 2018, 8:53 PM
apol added a subscriber: apol.May 27 2018, 10:22 PM

+1 to having this in kconfigwidgets. In fact most of it should be there already. We have a very similar code in kdevelop (and I know there's some at least in digikam, labplot and possibly krita).

@tfjellstrom Would you give this a try?

zhigalin accepted this revision.May 28 2018, 5:46 PM
zhigalin added a subscriber: broulik.

Indeed, maybe this class should be part of the KConfigWidgets framework? See https://api.kde.org/frameworks/kconfigwidgets/html/annotated.html for current list of classes - it already contains the color scheme stuff and is a tier3 framework (so many dependencies allowed).

@zhigalin Any comments from your side?

Sorry for the late reply, haven't noticed this review before.
As I said previously:

Now KDelelop also allows scheme selection, but I still think this should become a standard functionality, maybe something like KStandardAction::configureColors(KXmlGuiWindow *win);

And this is a reply by @broulik

If, and only if, we were to add something like that, we could add it to KXmlGui and have it provide a settings dialog in the settings menu where you also find eg. "Configure Toolbars".

The infrastructure is already there, KWin can even follow the colorscheme of an application to colorize the title bar accordingly. However, with KColorSchemeManager there's an easy-to-use class that applications that want to provide such functionality (eg. Krita, KDevelop, Gwenview, Dragon Player) > can add to their settings without further complicating the already insane customizability of our applications.

kate/katecolorschemechooser.h
39

KateColorSchemeChooser may be a bit misleading as there is also the Editor color scheme which is different...
Maybe it should be like KateGuiColorsChooser or KateWindowColorChooser?
The rest looks good to me.

This revision is now accepted and ready to land.May 28 2018, 5:46 PM
dhaumann added a subscriber: rempt.May 29 2018, 2:05 AM

@rempt Does Krita also allow changing the application color scheme? Would something like this be useful?

@zhigalin Usually KDE had the rule of thumb that a class should be in a library once there are two use cases. Strictly speaking, this is the case here.

rempt added a comment.May 29 2018, 6:42 AM

@rempt Does Krita also allow changing the application color scheme? Would something like this be useful?

Yes. We borrowed Digikam's code for that.

@rempt Is Digikams code the same as this?

So we have Krita, Digikam, KDevelop, Kate?

@rempt Is Digikams code the same as this?

I'm not sure what it is like now in digikam; Krita's code is completely different and doesn't use KColorSchemeManager.

So we have Krita, Digikam, KDevelop, Kate?

I tried the patch, I like the idea, now that I used it ;=)
I think centralizing that would be even nicer, but one can still remove this again if that goal is reached.
I will push this change before it rots away.

This revision was automatically updated to reflect the committed changes.
danielmu removed a subscriber: danielmu.