ScintillaOrg / lexilla

A library of language lexers for use with Scintilla
https://www.scintilla.org/Lexilla.html
Other
163 stars 59 forks source link

[CSS] Nesting and &-selector #210

Open jansalleine opened 8 months ago

jansalleine commented 8 months ago

Nesting and the &-selector are now supported in CSS and shoud be enabled for the "normal" CSS lexer too. https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_nesting/Using_CSS_nesting

Nesting check: https://github.com/ScintillaOrg/lexilla/blob/master/lexers/LexCSS.cxx#L111 &-selector check: https://github.com/ScintillaOrg/lexilla/blob/master/lexers/LexCSS.cxx#L370

jansalleine commented 8 months ago

I don't know how all this "pull request" stuff works on github. I created a .patch file for LexCSS.cxx, that someone could use as a starting point. I have tested this successfully with replacing the lexilla included in geany 2.0.0 with it. LexCSS.patch.zip

diff --git a/lexers/LexCSS.cxx b/lexers/LexCSS.cxx
index b50fbaf2..3940299f 100644
--- a/lexers/LexCSS.cxx
+++ b/lexers/LexCSS.cxx
@@ -119,13 +119,9 @@ static void ColouriseCssDoc(Sci_PositionU startPos, Sci_Position length, int ini
    CommentMode comment_mode = eCommentBlock;
    bool hasSingleLineComments = isScssDocument || isLessDocument || isHssDocument;

-   // must keep track of nesting level in document types that support it (SCSS/LESS/HSS)
-   bool hasNesting = false;
-   int nestingLevel = 0;
-   if (isScssDocument || isLessDocument || isHssDocument) {
-       hasNesting = true;
-       nestingLevel = NestingLevelLookBehind(startPos, styler);
-   }
+   // must keep track of nesting level in document types that support it (CSS/SCSS/LESS/HSS)
+   bool hasNesting = true;
+   int nestingLevel = NestingLevelLookBehind(startPos, styler);

    // "the loop"
    for (; sc.More(); sc.Forward()) {
@@ -341,6 +337,16 @@ static void ColouriseCssDoc(Sci_PositionU startPos, Sci_Position length, int ini
        else if (sc.ch == ')')
            insideParentheses = false;

+       // nested rule parent selector
+       if (sc.ch == '&') {
+           switch (sc.state) {
+           case SCE_CSS_DEFAULT:
+           case SCE_CSS_IDENTIFIER:
+               sc.SetState(SCE_CSS_TAG);
+               continue;
+           }
+       }
+
        // SCSS special modes
        if (hasVariables) {
            // variable name
@@ -366,19 +372,9 @@ static void ColouriseCssDoc(Sci_PositionU startPos, Sci_Position length, int ini
                    sc.SetState(SCE_CSS_VALUE);
                }
            }
-
-           // nested rule parent selector
-           if (sc.ch == '&') {
-               switch (sc.state) {
-               case SCE_CSS_DEFAULT:
-               case SCE_CSS_IDENTIFIER:
-                   sc.SetState(SCE_CSS_TAG);
-                   continue;
-               }
-           }
        }

-       // nesting rules that apply to SCSS and Less
+       // nesting rules that apply to CSS, SCSS and Less
        if (hasNesting) {
            // check for nested rule selector
            if (sc.state == SCE_CSS_IDENTIFIER && (IsAWordChar(sc.ch) || sc.ch == ':' || sc.ch == '.' || sc.ch == '#')) {
nyamatongwe commented 8 months ago

This causes the lexer to re-read the document from the start (NestingLevelLookBehind) to the update position before each update. This may be expensive on large documents and even lead to quadratic slowdowns when lexing incrementally. Re-reading was included for the pre-processors on the basis that it worsened performance only when choosing a pre-processor mode, not for standard CSS.

You could make the nesting behaviour opt-in with an additional property similar to lexer.css.scss.language or remember the nesting level for each line with SetLineState. See other files such as LexR.cxx for an example of using SetLineState and GetLineState.

jansalleine commented 8 months ago

I must admit I didn't look too deep into what the code actually does. After I saw that the if clause for the '&' selector was inside a (hasVariables) if clause I decided that either the code is kind of weird or I'm too much of a noob to get it – probably it's a bit of both.

The list of TODO comments seem to indicate that there's noone really caring at the moment. So I'll try my best to read more into this lexer nesting stuff and contribute if I'm able to.

Thanks for the hint with SetLineState.

nyamatongwe commented 8 months ago

Another problem with NestingLevelLookBehind that it assumes every { and } is about nesting even when inside comments or strings.