[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kwrite-devel
Subject:    Re: Patch for "selected item uses system background color,   not
From:       Matthew Woehlke <mw_triad () users ! sourceforge ! net>
Date:       2007-07-25 18:22:36
Message-ID: f884dd$mkb$1 () sea ! gmane ! org
[Download RAW message or body]

Mirko Stocker wrote:
> On Wednesday 25 July 2007 18:17:36 Matthew Woehlke wrote:
>> Awesome! Personally I would fix the fall-through case first, depending
>> on your comfort level with the fix you can post the updated patch for
>> review first (especially since I'm generally managing to look at your
>> patches, which is more (immediate) love then many patches get :-)).
> 
> I hope that isn't because my patches are bad, right? :)

Nah, I prefer another set of eyes seeing my patches except when I am 
very confident what they do (alas, often no one says anything about :-( 
). Like I said it's a comfort level thing, it's really up to you.

> And by the way, my patch for the dsNormal rendering, can I commit it?

I guess if no one objects. :-) However it doesn't seem to completely fix 
the issue. I did this:

Index: render/katerenderer.cpp
===================================================================
--- render/katerenderer.cpp     (revision 692445)
+++ render/katerenderer.cpp     (working copy)
@@ -427,6 +427,13 @@
    }

    if (range->layout()) {
+
+    if(range->textLine()->attributesList().empty()) {
+      kDebug()<<"Help! No attribute!"<<endl;
+      // had better not need this!
+ 
//paint.setPen(attribute(KateExtendedAttribute::dsNormal)->foreground().color());
+    }
+
      QVector<QTextLayout::FormatRange> additionalFormats;
      if (range->length() > 0) {
        // Draw the text :)

...and I see that happening pretty much every repaint. So while your 
patch *looks* like it should help, it seems it doesn't. Maybe something 
is messed up in my environment? What happens for you if you apply the above?

(Have you "discovered" IRC yet? ;-) )

>> Is that screenshot w/o your patch? Over here, with your
>> patch, I see the background for "Keyword" as pink, not blue.
> 
> Yes that's without my patch (I did it from my computer at the office).

Ok.

If you want to commit that one now that should be OK, but I would commit 
it without removing (changing would be OK) the TODO entry because the 
fall-through doesn't work yet. If you think you won't be able to fix 
that immediately, this is probably the right thing to do. Or you can fix 
that first and then commit, it's your decision :-).

-- 
Matthew
Microsoft: Expect the unexpected

_______________________________________________
KWrite-Devel mailing list
KWrite-Devel@kde.org
https://mail.kde.org/mailman/listinfo/kwrite-devel
[prev in list] [next in list] [prev in thread] [next in thread]