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