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

List:       kwrite-devel
Subject:    Re: Review Request: Python syntax string highlighting fixes
From:       "Matthew Woehlke" <mw_triad () users ! sourceforge ! net>
Date:       2009-09-08 22:50:36
Message-ID: 20090908225036.11289.95456 () localhost
[Download RAW message or body]



> On 2009-09-08 17:52:43, Matthew Woehlke wrote:
> > /trunk/KDE/kdelibs/kate/syntax/data/python.xml, line 382
> > <http://reviewboard.kde.org/r/1513/diff/2/?file=11087#file11087line382>
> > 
> > You do realize that by default this is the same as dsKeyword?
> 
> wrote:
> Operators are not keywords.

No, they are closer to dsOther. But  «defStyleNum="dsNormal" bold="1" » will *appear* the same as \
dsKeyword in the default configuration, so "Operator" will look like dsKeyword to people that have not \
changed dsKeyword to something other than the default 'the same as dsNormal, except bold'.


> On 2009-09-08 17:52:43, Matthew Woehlke wrote:
> > /trunk/KDE/kdelibs/kate/syntax/data/python.xml, line 396
> > <http://reviewboard.kde.org/r/1513/diff/2/?file=11087#file11087line396>
> > 
> > I realize you didn't change this, but there is a style for 'Base N Integers' (I forget the "dsXX" \
> > name offhand) that should probably be used for this, also for "Octal" and maybe "Complex" as well (or \
> > maybe dsFloat for Complex).
> 
> wrote:
> I've fixed this already in a later patch. I would link you to the relevant entry on anonsvn but it \
> seems to be broken at the moment.

Okay, I was going by just the bits that are here on RB.


> On 2009-09-08 17:52:43, Matthew Woehlke wrote:
> > /trunk/KDE/kdelibs/kate/syntax/data/python.xml, line 383
> > <http://reviewboard.kde.org/r/1513/diff/2/?file=11087#file11087line383>
> > 
> > Hard-coded colors are evil; in syntax files especially so. Have you considered if there is a better \
> > way?
> 
> wrote:
> A search through the syntax highlighters shows over 400 instances of manual suggestions of colours. I \
> therefore assert that this is the standard thing to do when adding a new language construct which Kate \
> does not automatically highlight; there is significant precedent for doing it this way at least. 

I can find lots of instances of hard-coded colors in applications (in general). For example, I recently \
fixed one in the vi-mode status bar, and a few in KAlarm (there remain others). And there are hard-coded \
colors in Thunderbird and QGit. That doesn't make it right to use hard-coded colors. That just makes it a \
/common/ bad practice.

While I will agree that it may be unavoidable to some extent at the moment ¹, it is best to make use of \
as many of kate's styles as possible before resorting to this, because it means you WILL have illegible \
text for some people that will have to go in and change it.

( ¹ That's why I didn't say "don't do this", I asked "[if] you considered if there is a better way". Your \
reply makes me think the answer to my question is "no".)

Please don't use "everyone else does it" as an excuse to perpetuate brokenness. (Again, I am not saying \
"you may not do this", just stop and think about the consequences and be sure you are convinced this is \
the lesser evil.)


- Matthew


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/1513/#review2269
-----------------------------------------------------------


On 2009-09-07 09:37:22, pag wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/1513/
> -----------------------------------------------------------
> 
> (Updated 2009-09-07 09:37:22)
> 
> 
> Review request for Kate.
> 
> 
> Summary
> -------
> 
> Update the Python syntax highlighting file:
> * Fully support string modifiers. Support was very weak and badly implemented before.
> * Highlighting the leading "u" on unicode strings as part of the string
> * Makes operators bold -- currently they're plain and easily ignored.
> 
> Am I OK to commit this?
> 
> 
> Diffs
> -----
> 
> /trunk/KDE/kdelibs/kate/syntax/data/python.xml 1020747 
> 
> Diff: http://reviewboard.kde.org/r/1513/diff
> 
> 
> Testing
> -------
> 
> Tested before and after against http://paul.giannaros.org/stuff/test_python_syntax.py 
> 
> 
> Screenshots
> -----------
> 
> String formatting before & after
> http://reviewboard.kde.org/r/1513/s/199/
> 
> 
> Thanks,
> 
> pag
> 
> 

_______________________________________________
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]