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

Reply via email to