Differential D5799

Rebase Less syntax highlighting on SCSS one
ClosedPublic
Actions

Authored by gszymaszek on May 10 2017, 6:17 PM.

Details

Reviewers
jtamate
dhaumann
Group Reviewers
Framework: Syntax Highlighting
Commits
R216:ca846fbda318: Rebased Less syntax highlighting on SCSS one
Summary

I’ve rebased Less syntax highlighting definitions on those from current SCSS syntax. Valid Less code is now mostly supported (I’ve tested this diff using various code snippets from “Features of the Less Language”); bug 369277 (“LessCSS syntax problem”) is solved.

Other notes:

Not supported:

  • invalid properties, like “padding-3” (properties cannot contain digits)
  • :nth-child parameters

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.
gszymaszek created this revision.May 10 2017, 6:17 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptMay 10 2017, 6:17 PM
Restricted Application added a subscriber: Frameworks. · View Herald Transcript
dhaumann added a subscriber: dhaumann.Jun 17 2017, 3:24 PM

@gszymaszek Could you also add a unit test to autotest/input/highlight.less ? This way, we can guarantee to not break the highlighting with future changes, which is very hard otherwise. The reference data is created by calling autotest/update-reference-data.sh in the build directory. It would be very helpful, if you update the patch again.

Besides that, in general this patch looks ok to me, although it is very hard to review (I don't know the language, and the diff is very big).

gszymaszek updated this revision to Diff 15814.Jun 24 2017, 10:18 AM

Added a test file (autotests/input/highlight.less).

jtamate requested changes to this revision.Aug 14 2017, 11:39 AM
jtamate added a subscriber: jtamate.

Hello,

To be able to compile with the current frameworks, I have had to remove the leading/trailing spaces:

"/5kde/frameworks/syntax-highlighting/data/syntax/less.xml" line 19 keyword with leading/trailing spaces: " align-content "
"/5kde/frameworks/syntax-highlighting/data/syntax/less.xml" line 20 keyword with leading/trailing spaces: " align-items "
"/5kde/frameworks/syntax-highlighting/data/syntax/less.xml" line 21 keyword with leading/trailing spaces: " align-self "
"/5kde/frameworks/syntax-highlighting/data/syntax/less.xml" line 22 keyword with leading/trailing spaces: " alignment-baseline "

Apart from that, it highlights without problems all the .less files I have tried.
Therefore, once the spaces are fixed, you have my ship it!

This revision now requires changes to proceed.Aug 14 2017, 11:39 AM
gszymaszek updated this revision to Diff 18296.EditedAug 17 2017, 5:03 PM
gszymaszek edited edge metadata.

Removed leading and trailing spaces inside XML tags (in less.xml), to make it possible to build the framework.

@jtamate, thank you for your review! If you think this diff is ready to ship, may I ask you to actually commit it (as I don’t have commit rights)?

dhaumann accepted this revision.Aug 17 2017, 8:28 PM

Looks good to me, I think we should include this change.

@gszymaszek Shall I commit this, or do you have commit rights? If you don't have commit rights, you should consider getting a KDE commit account.

Closed by commit R216:ca846fbda318: Rebased Less syntax highlighting on SCSS one (authored by gszymaszek). · Explain WhyAug 19 2017, 2:07 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
AMautotests/input/highlight.less (701 lines)
Mdata/syntax/less.xml (750 lines)

Diff 18391

View Options

autotests/input/highlight.less

Loading...
View Options

data/syntax/less.xml

Loading...