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

[Christoph Cullmann]

This revision was automatically updated to reflect the committed changes.
Closed by commit R216:43396e0a9773: Optimize many syntax highlighting files and 
fix the / char of SQL (authored by cullmann).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D11543?vs=31565=39719#toc

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11543?vs=31565=39719

REVISION DETAIL
  https://phabricator.kde.org/D11543

AFFECTED FILES
  data/syntax/prolog.xml
  data/syntax/pug.xml
  data/syntax/qml.xml
  data/syntax/r.xml
  data/syntax/rest.xml
  data/syntax/rhtml.xml
  data/syntax/ruby.xml
  data/syntax/sisu.xml
  data/syntax/sql-mysql.xml
  data/syntax/sql-postgresql.xml
  data/syntax/sql.xml
  data/syntax/tads3.xml
  data/syntax/tcl.xml
  data/syntax/template-toolkit.xml
  data/syntax/textile.xml
  data/syntax/varnishtest.xml
  data/syntax/varnishtest4.xml
  data/syntax/vhdl.xml
  data/syntax/wml.xml
  data/syntax/xmldebug.xml
  data/syntax/zsh.xml

To: nibags, #framework_syntax_highlighting, dhaumann
Cc: kwrite-devel, kde-frameworks-devel, dhaumann, michaelh, genethomas, 
kevinapavew, ngraham, bruns, demsking, cullmann, vkrause, sars

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

[Dominik Haumann]

dhaumann accepted this revision.
dhaumann added a comment.
This revision is now accepted and ready to land.
Restricted Application added a project: Kate.
Restricted Application edited subscribers, added: kde-frameworks-devel, 
kwrite-devel; removed: Frameworks.


  Thanks, changes look, please commit (after running make test) :-)

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  optimize_many_files (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D11543

To: nibags, #framework_syntax_highlighting, dhaumann
Cc: kwrite-devel, kde-frameworks-devel, dhaumann, michaelh, genethomas, 
kevinapavew, ngraham, bruns, demsking, cullmann, vkrause, sars, #frameworks

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

[Nibaldo González]

nibags updated this revision to Diff 31565.
nibags added a comment.


  - xharbour.xml is restored
  
  I have verified that the `Int` rule is not equivalent to ``

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11543?vs=30172=31565

BRANCH
  optimize_many_files (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D11543

AFFECTED FILES
  data/syntax/prolog.xml
  data/syntax/pug.xml
  data/syntax/qml.xml
  data/syntax/r.xml
  data/syntax/rest.xml
  data/syntax/rhtml.xml
  data/syntax/ruby.xml
  data/syntax/sisu.xml
  data/syntax/sql-mysql.xml
  data/syntax/sql-postgresql.xml
  data/syntax/sql.xml
  data/syntax/tads3.xml
  data/syntax/tcl.xml
  data/syntax/template-toolkit.xml
  data/syntax/textile.xml
  data/syntax/varnishtest.xml
  data/syntax/varnishtest4.xml
  data/syntax/vhdl.xml
  data/syntax/wml.xml
  data/syntax/xmldebug.xml
  data/syntax/zsh.xml

To: nibags, #framework_syntax_highlighting, dhaumann
Cc: dhaumann, #frameworks, michaelh, genethomas, ngraham, bruns, cullmann, 
vkrause

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

[Nibaldo González]

nibags updated this revision to Diff 30172.
nibags added a comment.


  Some changes are undone:
  
  - **prolog.xml** : Remove fallthrought, it goes back to RegExpr [1 context]. 
Use kateversion 3.4.
  - **rpmspec.xml** : All changes are undone: remove fallthrought [4 contexts]. 
Use kateversion 2.4.
  - **sql-mysql.xml** : It goes back to RegExpr, instead of WordDetect [2 
rules]. Use kateversion 3.4.
  - **sql-oracle.xml** : All changes are undone: It goes back to RegExpr, 
instead of WordDetect [1 rule]. Use kateversion 2.4.
  - **sql.xml** : It goes back to RegExpr, instead of WordDetect [1 rule]. Use 
kateversion 2.4.
  - **tcl.xml** : Remove fallthrought [2 contexts]. Use kateversion 2.4.
  - **varnishtest4.xml** : Remove DetectSpaces, it goes back to RegExpr [3 
rules]. Use kateversion 3.4.
  - **zsh.xml** : It goes back to RegExpr, instead of WordDetect [1 rule] & 
DetectIdentifier [2 rules]. Use kateversion 2.4.
  
  In `xmldebug.xml` is changed kateversion from 2.4 to 5.0; and the 
*sql_oracle* test files are restored.

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D11543?vs=30107=30172

BRANCH
  optimize_many_files (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D11543

AFFECTED FILES
  data/syntax/prolog.xml
  data/syntax/pug.xml
  data/syntax/qml.xml
  data/syntax/r.xml
  data/syntax/rest.xml
  data/syntax/rhtml.xml
  data/syntax/ruby.xml
  data/syntax/sisu.xml
  data/syntax/sql-mysql.xml
  data/syntax/sql-postgresql.xml
  data/syntax/sql.xml
  data/syntax/tads3.xml
  data/syntax/tcl.xml
  data/syntax/template-toolkit.xml
  data/syntax/textile.xml
  data/syntax/varnishtest.xml
  data/syntax/varnishtest4.xml
  data/syntax/vhdl.xml
  data/syntax/wml.xml
  data/syntax/xharbour.xml
  data/syntax/xmldebug.xml
  data/syntax/zsh.xml

To: nibags, #framework_syntax_highlighting, dhaumann
Cc: dhaumann, #frameworks, michaelh, genethomas, ngraham, cullmann, vkrause

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

[Nibaldo González]

nibags added a comment.


  Then, in such a case, it would be better to make only conservative changes 
such as:
  
  - Change StringDetect to DetectChar or Detect2Chars.
  - Change AnyChar for DetectChar.
  - Change RegExpr for StringDetect, DetectChar or Detect2Chars.
  
  It would be better to undo some changes from RegExpr to WordDetect, 
DetectSpaces or Fallthrough to avoid regressions in older versions of 
KTextEditor. Because I do not believe that the difference in performance is so 
great as to justify the changes and stop supporting old versions.

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

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

[Dominik Haumann]

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
>attribute="% italic predicates: w/ side effects" />
> - 
> -  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
>   beginRegion="block" insensitive="true"/>
> - endRegion="block" insensitive="true"/>
> + endRegion="block" insensitive="true"/>
>  

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
>  
> - extensions="*.sql;*.SQL;*.ddl;*.DDL" mimetype="text/x-sql" casesensitive="0" 
> author="Yury Lebedev (yurylebe...@mail.ru)" license="LGPL">
> + extensions="*.sql;*.SQL;*.ddl;*.DDL" mimetype="text/x-sql" casesensitive="0" 
> author="Yury Lebedev (yurylebe...@mail.ru)" license="LGPL">
>

Please raise to kateversion="5.0", since WordDetect was added later.

> xharbour.xml:491
>char1=">" />
> - 
> + 
>

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
>  
> - extensions="" mimetype="">
> + extensions="" mimetype="">
>

I believe DetectSpaces does not exist in kateview="2.4". Please raise to 
kateversion="5.0"

> zsh.xml:11
>  ]>
> - extensions="*.sh;*.zsh;.zshrc;.zprofile;.zlogin;.zlogout;.profile" 
> mimetype="application/x-shellscript" casesensitive="1" author="Jonathan 
> Kolberg (bulldo...@kubuntu-de.org)" license="LGPL">
> + 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

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

[Nibaldo González]

nibags edited the summary of this revision.

REPOSITORY
  R216 Syntax Highlighting

REVISION DETAIL
  https://phabricator.kde.org/D11543

To: nibags, #framework_syntax_highlighting
Cc: #frameworks, michaelh, genethomas, ngraham, cullmann, vkrause, dhaumann

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

[Nibaldo González]

nibags edited the summary of this revision.

REPOSITORY
  R216 Syntax Highlighting

REVISION DETAIL
  https://phabricator.kde.org/D11543

To: nibags, #framework_syntax_highlighting
Cc: #frameworks, michaelh, genethomas, ngraham, cullmann, vkrause, dhaumann

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

[Nibaldo González]

nibags edited the summary of this revision.
nibags added a reviewer: Framework: Syntax Highlighting.
nibags added a project: Framework: Syntax Highlighting.

REPOSITORY
  R216 Syntax Highlighting

REVISION DETAIL
  https://phabricator.kde.org/D11543

To: nibags, #framework_syntax_highlighting
Cc: #frameworks, michaelh, genethomas, ngraham, cullmann, vkrause, dhaumann

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

[Nibaldo González]

nibags created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.
nibags requested review of this revision.

REVISION SUMMARY
  Some suggested optimizations of D10621 .
  
  **Note:**
  

  
  Using KDE Frameworks 5.44, the above is only applied to the 'keywords' rules. 
  However, when you run `./bin/testhighlighter_test`,`./bin/folding_test` & 
`./bin/htmlhighlighter_test`, it also applies to the delimiters of the 
'WordDetect' rules. 
  This generates an error when I run testing binaries on the `test.sql_oracle` 
file, in the last line `end; ` (`` is changed to 
WordDetect and ';' is not delimiter). The test files with the problem are 
included.
  
  The only files where RegExpr is changed by WordDetect are `sql*.xml` and 
`rhtml.xml`. I do not know if it will be necessary to undo these changes or it 
is just a problem of the test generator.
  
  **Fix in SQL:**
  Fix bug in `sql-mysql.xml`, `sql-postgresql.xml` & `sql.xml` files: The 
single character '/' on a new line (rule: ``) is not 
highlighted, because it has conflict with the '/' keyword in the "operators" 
list. This rule is replaced by LineContinue (with column=0) and placed before 
the rule ``.
  
  - sql-mysql.xml
- [Line 481] RegExpr -> [Line 394] LineContinue.
  - sql-postgresql.xml
- [761] RegExpr -> [744] LineContinue.
  - sql.xml
- [914] RegExpr -> [897] LineContinue.
  
  **Files changed:**
  
  - prolog.xml
  - pug.xml
- [Line 36] RegExpr "\belse if\b" is replaced with "\belse\s+if\b". It is 
much more appropriate than using WordDetect, since it is JavaScript code (is 
Node.js)
  - qml.xml
  - r.xml
  - rest.xml
- [Lines 39-40] Merge RegExpr rules.
  - rhtml.xml
- [Lines 586-587] Merge RegExpr rules.
  - rpmspec.xml
  - ruby.xml
  - sisu.xml
  - sql-mysql.xml
  - sql-oracle.xml
  - sql-postgresql.xml
  - sql.xml
  - tads3.xml
  - tcl.xml
  - template-toolkit.xml
- Duplicate rules are deleted.
  - textile.xml
- [Lines 36-39] Merge RegExpr rules.
  - varnishtest.xml
  - varnishtest4.xml
  - vhdl.xml
- [Lines 511-514] Merge RegExpr rules.
  - wml.xml
  - xharbour.xml
- [Line 491] RegExpr ("\d+") -> Int
  - xmldebug.xml
- RegExpr ("\s+") -> DetectSpaces
  - zsh.xml
- RegExpr ("[A-Za-z_]\w*") -> DetectIdentifier

TEST PLAN
  I tried most of the changes to avoid problems.

REPOSITORY
  R216 Syntax Highlighting

BRANCH
  optimize_many_files (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D11543

AFFECTED FILES
  autotests/folding/test.sql_oracle.fold
  autotests/html/test.sql_oracle.html
  autotests/reference/test.sql_oracle.ref
  data/syntax/prolog.xml
  data/syntax/pug.xml
  data/syntax/qml.xml
  data/syntax/r.xml
  data/syntax/rest.xml
  data/syntax/rhtml.xml
  data/syntax/rpmspec.xml
  data/syntax/ruby.xml
  data/syntax/sisu.xml
  data/syntax/sql-mysql.xml
  data/syntax/sql-oracle.xml
  data/syntax/sql-postgresql.xml
  data/syntax/sql.xml
  data/syntax/tads3.xml
  data/syntax/tcl.xml
  data/syntax/template-toolkit.xml
  data/syntax/textile.xml
  data/syntax/varnishtest.xml
  data/syntax/varnishtest4.xml
  data/syntax/vhdl.xml
  data/syntax/wml.xml
  data/syntax/xharbour.xml
  data/syntax/xmldebug.xml
  data/syntax/zsh.xml

To: nibags
Cc: #frameworks, michaelh, ngraham