D15518: Editor - Line highlighting color fix

Tomaz Canabrava noreply at phabricator.kde.org
Mon Sep 17 08:52:01 BST 2018


tcanabrava added inline comments.

INLINE COMMENTS

> editor.h:148
>  			QPainter painter(viewport());
> -			painter.fillRect(currentLineRect(), QBrush(LINE_HIGHLIGHT_COLOR));
> +			QColor lineColor, errorColor, wordColor; // background color for current line, error highlighting and word highlighting
> +			QColor bgColor = this->palette().brush(this->backgroundRole()).color();

Declare the variables when you are going to use them.

> editor.h:149
> +			QColor lineColor, errorColor, wordColor; // background color for current line, error highlighting and word highlighting
> +			QColor bgColor = this->palette().brush(this->backgroundRole()).color();
> +			lineColor.setHsv(

don't use this->, this is not java.
access the private variables directly.

> editor.h:155-164
> +			errorColor.setHsv(
> +			ERROR_HIGHLIGHT_COLOR.hue(),
> +			bgColor.saturation() + EXTRA_SATURATION,
> +			bgColor.value()); // background for errors
> +
> +			wordColor.setHsv(
> +			WORD_HIGHLIGHT_COLOR.hue(),

Why are you setting the values of errorColor wordColor in a if-branch that you will not use?

> editor.h:160-164
> +			wordColor.setHsv(
> +			WORD_HIGHLIGHT_COLOR.hue(),
> +			bgColor.saturation() + EXTRA_SATURATION,
> +			bgColor.value()); //word highlighting background
> +			painter.fillRect(currentLineRect(), QBrush(lineColor)); //WAS - painter.fillRect(currentLineRect(), QBrush(LINE_HIGHLIGHT_COLOR));

create a lambda so you don't need to pass bgColor.saturation() / value() three times.

REPOSITORY
  R337 KTurtle

REVISION DETAIL
  https://phabricator.kde.org/D15518

To: mvalentino, tcanabrava, #kde_edu
Cc: kde-edu, narvaez, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-edu/attachments/20180917/5ffb115f/attachment-0001.html>


More information about the kde-edu mailing list