From kwrite-devel Tue Sep 08 22:50:36 2009 From: "Matthew Woehlke" Date: Tue, 08 Sep 2009 22:50:36 +0000 To: kwrite-devel Subject: Re: Review Request: Python syntax string highlighting fixes Message-Id: <20090908225036.11289.95456 () localhost> X-MARC-Message: https://marc.info/?l=kwrite-devel&m=125245026016974 > On 2009-09-08 17:52:43, Matthew Woehlke wrote: > > /trunk/KDE/kdelibs/kate/syntax/data/python.xml, line 382 > > > > > > 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 > > > > > > 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 > > > > > > 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