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

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

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.

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.

xharbour.xml is restored

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

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

			Path	Packages
M			data/syntax/prolog.xml (8 lines)
M			data/syntax/pug.xml (2 lines)
M			data/syntax/qml.xml (4 lines)
M			data/syntax/r.xml (6 lines)
M			data/syntax/rest.xml (7 lines)
M			data/syntax/rhtml.xml (91 lines)
M			data/syntax/ruby.xml (74 lines)
M			data/syntax/sisu.xml (166 lines)
M			data/syntax/sql-mysql.xml (5 lines)
M			data/syntax/sql-postgresql.xml (11 lines)
M			data/syntax/sql.xml (4 lines)
M			data/syntax/tads3.xml (4 lines)
M			data/syntax/tcl.xml (6 lines)
M			data/syntax/template-toolkit.xml (35 lines)
M			data/syntax/textile.xml (19 lines)
M			data/syntax/varnishtest.xml (30 lines)
M			data/syntax/varnishtest4.xml (56 lines)
M			data/syntax/vhdl.xml (12 lines)
M			data/syntax/wml.xml (8 lines)
M			data/syntax/xmldebug.xml (164 lines)
M			data/syntax/zsh.xml (6 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	30107	cec7635		Mar 21 2018, 10:07 AM	★	★
Diff 2	30172	cec7635		Mar 22 2018, 1:49 AM	★	★
Diff 3	31565	cec7635	- xharbour.xml is restored	Apr 7 2018, 7:34 AM	★	★
Diff 4	39719	bffb66c	R216:43396e0a9773c256a49e15fd08793b6cec86172d	Aug 14 2018, 3:07 PM	★	★

Commit	Tree	Parents	Author	Summary	Date
baed2b7f223d	206fbc0d0458	ab6eca7679be	Nibaldo González	xharbour.xml is restored	Apr 7 2018, 7:28 AM
ab6eca7679be	87bf5f0c2f93	3edca938b303	Nibaldo González	Some changes are undone: (Show More…)	Mar 22 2018, 1:45 AM
3edca938b303	3a5c0aa34aae	cec763566ca4	Nibaldo González	Optimize many syntax highlighting files and fix the '/' char of SQL (Show More…)	Mar 21 2018, 9:57 AM