ccw-ide / ccw

Counterclockwise is an Eclipse plugin helping developers write Clojure code
https://github.com/laurentpetit/ccw/wiki/GoogleCodeHome
Eclipse Public License 1.0
220 stars 50 forks source link

Problem with dark editor theme #799

Open asampal opened 9 years ago

asampal commented 9 years ago

If the editor background color is set to black, it remains white except for the line number columns.

Eclipse 4.5, Windows 8.1 ccw.core 0.32.0.201506262242

laurentpetit commented 9 years ago

Reproduced. @arichiardi do you think you'll have the possibiity to take a look, or at least point me in the right direction?

arichiardi commented 9 years ago

I think yes i will try this weekend, it should not be some issue with tokens, probably the background pref not read/set correctly

laurentpetit commented 9 years ago

Hello, 

I have found the time to look at it and I think I have a solution. I will submit a PR for review, if you're ok with this?

— Envoyé avec Mailbox

On Sat, Jun 27, 2015 at 5:18 PM, Andrea Richiardi notifications@github.com wrote:

I think yes i will try this weekend, it should not be some issue with tokens, probably the background pref not read/set correctly

Reply to this email directly or view it on GitHub: https://github.com/laurentpetit/ccw/issues/799#issuecomment-116087368

arichiardi commented 9 years ago

Oh yes please go ahead, i did not have time to have a look but i will test it for sure

laurentpetit commented 9 years ago

@arichiardi the fix is in this commit, can you review it? https://github.com/laurentpetit/ccw/commit/6fd012a38e6fa3ee3b90b9cc78beb49d22385124

asampal commented 9 years ago

The fix works, but there is still a one character wide column that remains white, separating the line number area from the editing area.

laurentpetit commented 9 years ago

@asampal indeed, will work on this

arichiardi commented 9 years ago

After having debugged ClojureTokenScanner, I discovered that it was not related... Probably folding is the culprit...https://stackoverflow.com/questions/12207637/eclipse-juno-how-to-change-background-of-code-folding-regions

arichiardi commented 9 years ago

Another related issue: https://stackoverflow.com/questions/12490672/vertical-white-line-on-eclipse

@asampal could you try these two workarounds?

arichiardi commented 9 years ago

And finally: https://bugs.eclipse.org/bugs/show_bug.cgi?id=194313

laurentpetit commented 9 years ago

Yes, folding seems to be the issue. JDT does not have the problem.

arichiardi commented 9 years ago

Sorry I have posted a workaround for CDT, will try something better, for example I can try emulating what JDT is doing

asampal commented 9 years ago

Toggling the folding functionality didn't fix this for me. Closed the editors and re-opened, but no go.

laurentpetit commented 9 years ago

@arichiardi I'd like to focus on what's required to release a new stable version. This issue seems like an embarrassing regression to me. Better to fix it soon. Can you give it another try?

arichiardi commented 9 years ago

Yes ok I will focus on this then

laurentpetit commented 9 years ago

thanks a lot!

arichiardi commented 9 years ago

Don't mention it :smile: !

laurentpetit commented 9 years ago

Just to refresh my memory, I gave the current stable version a try, and it does not have the problem, indeed. I tried to disable as many options as possible related to folding, but the problem remains the same, so it may not be related to the same problem people have reported on Stack Overflow.

Will try to bisect to find where the problem occurs.

laurentpetit commented 9 years ago

OK, so bisecting confirms that fcc360c is the first commit introducing the problem

laurentpetit commented 9 years ago

Well, big commit, hard to tell where the problem lies. Will try another approach and put some breakpoints in the StyledText

arichiardi commented 9 years ago

Will debug too, I remember I did change nothing about the tokens...it is weird because it looks like that stripe is not even within buffer range...I will help you out and sorry about it..

arichiardi commented 9 years ago

initializeViewerColors looks suspicious for instance, maybe it does not take the black background color

arichiardi commented 9 years ago

Another road I am considering: https://github.com/guari/eclipse-ui-theme/issues/7

arichiardi commented 9 years ago

I put here the SWT spy log:

AnnotationRulerColumn$5 {}@140230138540928
    Style: NO_FOCUS | LEFT_TO_RIGHT | DOUBLE_BUFFERED 
    Layout Data: null
    Bounds: Rectangle {44, 0, 9, 206}

Children:

Peers:
    *AnnotationRulerColumn$5 {}@140230138540928 Layout Data: null Bounds: Rectangle {44, 0, 9, 206}
    AnnotationRulerColumn$5 {}@140230141853568 Layout Data: null Bounds: Rectangle {0, 0, 12, 206}
    LineNumberRulerColumn$4 {}@140230140489248 Layout Data: null Bounds: Rectangle {12, 0, 32, 206}

Parent Tree:
[CUT]

Canvas {}@139916372127008
                                                             Style: LEFT_TO_RIGHT | DOUBLE_BUFFERED 
                                                             Bounds: Rectangle {0, 0, 954, 276}
                                                             Layout: org.eclipse.jface.text.source.SourceViewer$RulerLayout@4a767290
                                                             LayoutData: null
                                                            CompositeRuler$CompositeRulerCanvas {}@139916355542160
                                                                 Style: LEFT_TO_RIGHT | DOUBLE_BUFFERED 
                                                                 Bounds: Rectangle {0, 0, 29, 258}
                                                                 Layout: org.eclipse.jface.text.source.CompositeRuler$RulerLayout@76b6b86f
                                                                 LayoutData: null
arichiardi commented 9 years ago

I am digging deep, basically the setBackground that follows correctly shows {255,0,0} the red I chose: selection_024

This is clear because we explicitely call styledText.setBacground in initializeViewerColors. But when AnnotationRulerColumn.redraw() is triggered, the background color on its fCanvas is {215,215,215}, which is that light gray you see...

Now, why in the world the color goes in the StyledText and not in the AnnotationRulerColumn I don't know...

laurentpetit commented 9 years ago

Yeah, I also already spent a night on this yesterday without success. But I'm stubborn, and I hope we'll find the problem. We may also learn things about how swt / jface / viewers work, along the way. 

— Envoyé avec Mailbox

On Wed, Jul 29, 2015 at 10:58 AM, Andrea Richiardi notifications@github.com wrote:

I am digging deep, basically the setBackground that follows correctly shows {255,0,0} the red I chose: selection_024 But when AnnotationRulerColumn.redraw() is triggered, the background color on its fCanvas is {215,215,215}, which is that light gray you see...

Now, why in the world the color goes in the StyledText and not in the AnnotationRulerColumn I don't know...

Reply to this email directly or view it on GitHub: https://github.com/laurentpetit/ccw/issues/799#issuecomment-125885515

arichiardi commented 9 years ago

By the way it still happens in my Mars with jdt...have you tried to put a color that contrasts your theme color? We'll better report this in here.

selection_025

EDIT: restarting basically solves the problem...it looks like there is some problem with live changes only. selection_026

laurentpetit commented 9 years ago

Well, on my Mars, java editor does not seem to have problems, while clojure editor does.

arichiardi commented 9 years ago

Could also be OS related, or Eclipse related, I am running on linux and y target platform is kepler atm.

laurentpetit commented 9 years ago

Indeed. I think I can probably release with this problem anyway. It will not be too late to fix it later, and it's kind of a shame that I'm holding back all the good stuff that's been done since January !

2015-07-29 21:01 GMT+02:00 Andrea Richiardi notifications@github.com:

Could also be OS related, or Eclipse related, I am running on linux and y target platform is kepler atm.

— Reply to this email directly or view it on GitHub https://github.com/laurentpetit/ccw/issues/799#issuecomment-126060016.

Laurent Petit

laurentpetit commented 9 years ago

Here's what I have here, on OS X, running Mars : screenshot 2015-07-29 21 15 05

arichiardi commented 9 years ago

That's true ;)

On July 29, 2015 9:14:10 PM GMT+02:00, Laurent Petit notifications@github.com wrote:

Indeed. I think I can probably release with this problem anyway. It will not be too late to fix it later, and it's kind of a shame that I'm holding back all the good stuff that's been done since January !

2015-07-29 21:01 GMT+02:00 Andrea Richiardi notifications@github.com:

Could also be OS related, or Eclipse related, I am running on linux and y target platform is kepler atm.

— Reply to this email directly or view it on GitHub

https://github.com/laurentpetit/ccw/issues/799#issuecomment-126060016.

Laurent Petit


Reply to this email directly or view it on GitHub: https://github.com/laurentpetit/ccw/issues/799#issuecomment-126063034

-ar

arichiardi commented 9 years ago

Even with folding, this problem is still there...very weird.

arichiardi commented 9 years ago

I mean in my branch folding-feature

arichiardi commented 9 years ago

I have randomly tried commit ae2e517 and it still has the problem, it was older than fcc360c88dfe5918681321a91c5eb2595eb4182e

arichiardi commented 9 years ago

In my Mars with Color Theme installed I went to Text Editors and reset to default colors...and...

selection_002

It looks like this bug is hidden in JDT, but it is still there...but still, we should try to fix it in CCW first.

arichiardi commented 9 years ago

Good news! I solved the problem!

selection_003

arichiardi commented 9 years ago

It will be in thn folding-feature because it is directly related with it, difficult to extrapolate...

laurentpetit commented 9 years ago

Good news indeed! What was the problem?

arichiardi commented 9 years ago

When you change the background basically, because of the above-mentioned bug, you need to toggle the folding on PrjectionViewer. I am planning to do it automagically thanks to your whiteboard pattern and listening to the background preference.

laurentpetit commented 9 years ago

(y)

arichiardi commented 9 years ago

Actually just listening to the background should suffice..