D11543: Optimize many syntax highlighting files and fix the '/' char of SQL

Dominik Haumann noreply at phabricator.kde.org
Wed Mar 21 21:11:36 UTC 2018


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 (yurylebedev at 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 (yurylebedev at 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 (bulldog98 at 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 (bulldog98 at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180321/f2fcda57/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list