Differential D11543

Optimize many syntax highlighting files and fix the '/' char of SQL
ClosedPublic
Actions

Authored by nibags on Mar 21 2018, 10:07 AM.

Details

Reviewers
dhaumann
Group Reviewers
Framework: Syntax Highlighting
Commits
R216:43396e0a9773: Optimize many syntax highlighting files and fix the '/' char of SQL
Summary

Some suggested optimizations of D10621.

Note:

<keywords additionalDeliminator="@" weakDeliminator=";" />

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; (<RegExpr String="\bend\b" ../> is changed to WordDetect and ';' is not delimiter). The test files with the problem are included.

The only files where RegExpr is changed for WordDetect (with the delimiters modified) 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: <RegExpr String="^/$" ../>) 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 <keyword String="operators" ../>.

  • 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.

Diff Detail

Repository
R216 Syntax Highlighting
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
nibags created this revision.Mar 21 2018, 10:07 AM
Restricted Application added a project: Frameworks. · View Herald TranscriptMar 21 2018, 10:07 AM
Restricted Application added a subscriber: Frameworks. · View Herald Transcript
nibags requested review of this revision.Mar 21 2018, 10:07 AM
nibags edited the summary of this revision. (Show Details)Mar 21 2018, 10:09 AM
nibags added a reviewer: Framework: Syntax Highlighting.
nibags added a project: Framework: Syntax Highlighting.
nibags edited the summary of this revision. (Show Details)Mar 21 2018, 10:12 AM
nibags edited the summary of this revision. (Show Details)Mar 21 2018, 10:34 AM
dhaumann requested changes to this revision.Mar 21 2018, 9:11 PM
dhaumann added a subscriber: dhaumann.

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.

data/syntax/prolog.xml
508

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.

data/syntax/sql-oracle.xml
2054 ↗(On Diff #30107)

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)?

data/syntax/sql.xml
8

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

data/syntax/xharbour.xml
491 ↗(On Diff #30107)

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...

data/syntax/xmldebug.xml
3

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

data/syntax/zsh.xml
11

Please increase kateversion="5.0".

This revision now requires changes to proceed.Mar 21 2018, 9:11 PM
nibags added a comment.Mar 21 2018, 9:48 PM

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.

nibags updated this revision to Diff 30172.Mar 22 2018, 1:49 AM

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.

nibags updated this revision to Diff 31565.Apr 7 2018, 7:34 AM
  • xharbour.xml is restored

I have verified that the Int rule is not equivalent to <RegExpr String="\d+".../>

dhaumann accepted this revision.Aug 13 2018, 9:10 AM

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

This revision is now accepted and ready to land.Aug 13 2018, 9:10 AM
Restricted Application added a project: Kate. · View Herald TranscriptAug 13 2018, 9:10 AM
Restricted Application edited subscribers, added: kde-frameworks-devel, kwrite-devel; removed: Frameworks. · View Herald Transcript
Closed by commit R216:43396e0a9773: Optimize many syntax highlighting files and fix the '/' char of SQL (authored by cullmann). · Explain WhyAug 14 2018, 3:07 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 39719

View Options

data/syntax/prolog.xml

Loading...
View Options

data/syntax/pug.xml

Loading...
View Options

data/syntax/qml.xml

Loading...
View Options

data/syntax/r.xml

Loading...
View Options

data/syntax/rest.xml

Loading...
View Options

data/syntax/rhtml.xml

Loading...
View Options

data/syntax/ruby.xml

Loading...
View Options

data/syntax/sisu.xml

Loading...
View Options

data/syntax/sql-mysql.xml

Loading...
View Options

data/syntax/sql-postgresql.xml

Loading...
View Options

data/syntax/sql.xml

Loading...
View Options

data/syntax/tads3.xml

Loading...
View Options

data/syntax/tcl.xml

Loading...
View Options

data/syntax/template-toolkit.xml

Loading...
View Options

data/syntax/textile.xml

Loading...
View Options

data/syntax/varnishtest.xml

Loading...
View Options

data/syntax/varnishtest4.xml

Loading...
View Options

data/syntax/vhdl.xml

Loading...
View Options

data/syntax/wml.xml

Loading...
View Options

data/syntax/xmldebug.xml

Loading...
View Options

data/syntax/zsh.xml

Loading...