[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]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic