dhaumann requested changes to this revision. dhaumann added a comment. This revision now requires changes to proceed.
Cool! The patch already looks pretty good. Please address or comment on my questions. And for a final round, I would like to have a review by another reviewer, since this patch is quite big, and I would like to avoid introducing regressions. INLINE COMMENTS > prolog.xml:508 > <Detect2Chars column="0" char="#" char1="!" context="1-comment" > attribute="% italic predicates: w/ side effects" /> > - <!-- else fallthrough (workaround broken fallthrough) --> > - <RegExpr String="&any;" lookAhead="true" context="clause" > attribute="Syntax Error" /> So fallthrough is not broken anymore? I am especially asking, since KTextEditor still uses its own highlighting implementation. Meaning that it may work in KSyntaxHighlighting, but it may break KTextEditor. > sql-oracle.xml:2054 > <StringDetect attribute="Keyword" context="#stay" String="begin" > beginRegion="block" insensitive="true"/> > - <RegExpr attribute="Keyword" context="#stay" String="\bend\b" > endRegion="block" insensitive="true"/> > + <WordDetect attribute="Keyword" context="#stay" String="end" > endRegion="block" insensitive="true"/> > <keyword attribute="Keyword" String="keywords" context="#stay"/> Doesn't this introduce a regression, as you noted yourself? See updated test case? Or did you find a bug? How does the KTextEditor implementation behave (Kate, KWrite)? > sql.xml:8 > <!-- kate: space-indent on; indent-width 2; replace-tabs on; --> > -<language name="SQL" version="3" kateversion="2.4" section="Database" > extensions="*.sql;*.SQL;*.ddl;*.DDL" mimetype="text/x-sql" casesensitive="0" > author="Yury Lebedev (yurylebe...@mail.ru)" license="LGPL"> > +<language name="SQL" version="4" kateversion="2.4" section="Database" > extensions="*.sql;*.SQL;*.ddl;*.DDL" mimetype="text/x-sql" casesensitive="0" > author="Yury Lebedev (yurylebe...@mail.ru)" license="LGPL"> > <highlighting> Please raise to kateversion="5.0", since WordDetect was added later. > xharbour.xml:491 > <Detect2Chars attribute="Operator" context="#stay" char="-" > char1=">" /> > - <RegExpr attribute="Number" context="#stay" String="\d+" /> > + <Int attribute="Number" context="#stay" /> > </context> Strictly speaking, the Int rule also matches negative numbers, whereas the previous rule only matched positive numbers. I am not sure what's correct, though... > xmldebug.xml:3 > <!DOCTYPE language SYSTEM "language.dtd"> > -<language version="7" kateversion="2.4" name="XML (Debug)" section="Markup" > extensions="" mimetype=""> > +<language version="8" kateversion="2.4" name="XML (Debug)" section="Markup" > extensions="" mimetype=""> > <highlighting> I believe DetectSpaces does not exist in kateview="2.4". Please raise to kateversion="5.0" > zsh.xml:11 > ]> > -<language name="Zsh" version="2" kateversion="2.4" section="Scripts" > extensions="*.sh;*.zsh;.zshrc;.zprofile;.zlogin;.zlogout;.profile" > mimetype="application/x-shellscript" casesensitive="1" author="Jonathan > Kolberg (bulldo...@kubuntu-de.org)" license="LGPL"> > +<language name="Zsh" version="3" kateversion="2.4" section="Scripts" > extensions="*.sh;*.zsh;.zshrc;.zprofile;.zlogin;.zlogout;.profile" > mimetype="application/x-shellscript" casesensitive="1" author="Jonathan > Kolberg (bulldo...@kubuntu-de.org)" license="LGPL"> > Please increase kateversion="5.0". REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D11543 To: nibags, #framework_syntax_highlighting, dhaumann Cc: dhaumann, #frameworks, michaelh, genethomas, ngraham, cullmann, vkrause