Review IconBorder
ClosedPublic

Authored by loh.tar on Apr 20 2019, 12:29 PM.

Details

Summary

The only two included changes noticeable by the user are:

  • Use line number color for unfolded triangle to be less intrusive
  • Move the line modification hint away from the edit area between the line numbers and the folding area to be less irritating when cursor is in first column

Most of the changes in this patch could you see as code cosmetic or an attempt to make it more maintenance-friendly.

  • Cleanup, avoid redundancies and nesting
  • Rename members, make them unified named
  • Give each area an own value name
  • Fix bookmark pixmap painting
  • Simplify "additional folding highlighting", there is now the slightly gradient gone, bad?
  • Remove m_oldBackgroundColor, can't see any benefit
  • Make positionToArea() and sizeHint() less error prone in case of design changes
Test Plan

Pls try it, but I'm not sure if I'm done with it

Diff Detail

Lint
Lint Skipped
Unit
Unit Tests Skipped
loh.tar created this revision.Apr 20 2019, 12:29 PM
Restricted Application added projects: Kate, Frameworks. · View Herald TranscriptApr 20 2019, 12:29 PM
Restricted Application added subscribers: kde-frameworks-devel, kwrite-devel. · View Herald Transcript
loh.tar requested review of this revision.Apr 20 2019, 12:29 PM
loh.tar edited the summary of this revision. (Show Details)Apr 20 2019, 12:34 PM
loh.tar edited the test plan for this revision. (Show Details)
dhaumann accepted this revision.Apr 20 2019, 11:35 PM
dhaumann added a subscriber: dhaumann.

I like the visual change, let's give it a try. Thanks!

This revision is now accepted and ready to land.Apr 20 2019, 11:35 PM
loh.tar updated this revision to Diff 56644.Apr 21 2019, 7:00 AM
  • Fix missing printed background in proper theme color
  • Fix scroll past end of document
  • Fix less pushy paint unfolded icon in not dark themes and don't try to use currentLineNumberColor, the folded icon gets also not highligted
  • Some more cosmetic

loh.tar updated this revision to Diff 56693.Apr 22 2019, 4:45 AM
loh.tar edited the summary of this revision. (Show Details)
  • Add margin to the edit area, but I'm not sure if it should be done here. I guess renderer should do it
  • Simplify "additional folding highlighting", there is now the slightly gradient gone, bad?
  • Rename backGroundColor->iconBarColor to fit orig name, but that name sounds to me as it's only for the icon area of the border

loh.tar updated this revision to Diff 56694.Apr 22 2019, 5:40 AM
loh.tar edited the summary of this revision. (Show Details)
  • Fix bookmark pixmap painting

Left before, right with fix

loh.tar edited the test plan for this revision. (Show Details)Apr 22 2019, 5:43 AM

@dhaumann Beside these annotation stuff I think I'm done with this now. Should you, or someone else, not stop me in the next few days I may treat this as OK and push it.
There are still some issues in the mouse move handling but will try to fix that in some other patch

Did you test that the annotation border still works? You can do so in KDevelop by invoking git blame.

Besides I still gave no issues with this patch, except that I did not test myself. If you say there are mouse move issues, then these issues should be fixed asap :)

Did you test that the annotation border still works? You can do so in KDevelop by invoking git blame.

:-/ ...OK, thanks

Besides I still gave no issues with this patch, except that I did not test myself. If you say there are mouse move issues, then these issues should be fixed asap :)

It's no new issue..so it's not so urgent :P

Well, issues that were already there before should not hinder this patch of course! If you think this is good enough, please go on.

cullmann requested changes to this revision.Apr 23 2019, 5:53 PM
cullmann added a subscriber: cullmann.

Unfortunately, the annotation stuff regressed.
I tried KDevelop, right click on text => Git -> Annotation...
See pre-patch and post-patch pictures below, the too small one is the post patch one.


This revision now requires changes to proceed.Apr 23 2019, 5:53 PM

I am not sure one can delay the updateGeometry stuff until one paints.

loh.tar updated this revision to Diff 56839.Apr 23 2019, 7:20 PM
loh.tar edited the test plan for this revision. (Show Details)

try to fix annotation issue

completely untested :-/

cullmann accepted this revision.Apr 23 2019, 7:34 PM

This works for me fine with KDevelop.
Please commit.
Thanks

This revision is now accepted and ready to land.Apr 23 2019, 7:34 PM
This revision was automatically updated to reflect the committed changes.
dfaure added a subscriber: dfaure.Apr 24 2019, 7:06 AM

This commit appears to have introduced a unittest regression

FAIL! : InlineNoteTest::testInlineNote() Compared values are not the same

Actual   (newCoordCol04): QPoint(51,1)
Expected (coordCol04)   : QPoint(33,1)
Loc: [/home/jenkins/workspace/Frameworks/ktexteditor/kf5-qt5 SUSEQt5.10/autotests/src/inlinenote_test.cpp(171)]

https://build.kde.org/job/Frameworks/view/Platform%20-%20SUSEQt5.10/job/ktexteditor/job/kf5-qt5%20SUSEQt5.10/98/testReport/junit/projectroot/autotests/inlinenote_test/

:( Sorry, I didn't run them again, just tried out if it works in KDevelop.

cullmann reopened this revision.Apr 24 2019, 7:15 AM

loh.tar, can you take a look?
If it is too complex to fix easily, we can still revert this and apply it again later together with a fix.
Thanks for pointing the CI fail out David!

This revision is now accepted and ready to land.Apr 24 2019, 7:15 AM
This revision was automatically updated to reflect the committed changes.