This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/115926/

On February 20th, 2014, 5:40 p.m. EST, Matthew Woehlke wrote:

part/utils/katedefaultcolors.cpp (Diff revision 2)
		68	case SearchHighlight:
		69	return QColor(Qt::yellow);
		70	case ReplaceHighlight:
		71	return QColor(Qt::green);

Can someone remind me what was wrong with {Neutral,Positive}Background here? Maybe not the place to change it in this patch but I do feel a little itchy that the comments what these "should" be are now gone too.

On February 20th, 2014, 5:54 p.m. EST, Dominik Haumann wrote:

Afaik, these colors are so decent that it somehow wasn't bold enough. But I may be wrong.

On February 20th, 2014, 5:57 p.m. EST, Milian Wolff wrote:

re-added the comment that this should/must be adapted to the color scheme (which is the whole point of why I started working on this).

On February 20th, 2014, 6:05 p.m. EST, Matthew Woehlke wrote:

@Dominik, that sounds suspiciously like my own hazy recollection, it's probably right.

@Milian, maybe these should come from a tint (or even a blend) of the foreground color the same way the scheme background colors do, but with a stronger tint applied. (FWIW, since this reminded me to go and fix my scheme so these are actually legible, what I ended up going with was a somewhat modestly tint of Active. I liked that better than Neutral, but my Active is of a cyan hue; for schemes that have a magenta Active that might not work.)

On February 20th, 2014, 6:09 p.m. EST, Milian Wolff wrote:

As I said, I don't want to do this in this patch, but will look into auto-tinting in a follow-up commit.

On February 21st, 2014, 1:03 a.m. EST, Christoph Cullmann wrote:

Yeah, the color scheme variants were not visible enough in most KDE provided color schemes at all.

@Milian, sorry for being unclear; I also didn't mean to do it in this patch :-). Apologies for the misunderstanding.

@Christoph, thanks for confirming.

- Matthew

On February 20th, 2014, 6:10 p.m. EST, Milian Wolff wrote:

Review request for Kate and Christoph Cullmann.

By Milian Wolff.

Updated Feb. 20, 2014, 6:10 p.m.

Repository: kate

Description

Instead of copy'n'pasting the generation of default colors, we put that into a
central place and generate the colors there.

Building on top of this patch, I plan to add some auto-tinting code as I use in
KDevelop for the currently hardcoded colors (see Qt::yellow & friends).

Please accept this for KDE4, as it's a bug fix for people like me.

Testing

Still looks as before to my eyes. Dunno if I regressed anything?

Diffs

part/CMakeLists.txt (737f325)
part/schema/kateschemaconfig.cpp (541be03)
part/utils/kateconfig.cpp (65e8e8e)
part/utils/katedefaultcolors.h (PRE-CREATION)
part/utils/katedefaultcolors.cpp (PRE-CREATION)

View Diff