Subject: Re: bug#53203: Comment with lots of color codes crashes or hangs emacs in scss-mode

From: Lars Ingebrigtsen <[email-masked]>
Date: Sat, 14 May 2022 14:38:21 +0200

OK, I think I understand most of the regexp now, but it's a strange mix
of strictness and sloppiness.

    (,(concat
       "^[ \t]*\\("
       (if (not sassy)
           ;; We don't allow / as first char, so as not to
           ;; take a comment as the beginning of a selector.
           "[^@/:{}() \t\n][^:{}()]*"
         ;; Same as for non-sassy except we do want to allow { and }
         ;; chars in selectors in the case of #{$foo}
         ;; variable interpolation!
         (concat "\\(?:" scss--hash-re
                 "\\|[^@/:{}() \t\n#]\\)"
                 "[^:{}()#]*\\(?:" scss--hash-re "[^:{}()#]*\\)*"))
       ;; Even though pseudo-elements should be prefixed by ::, a
       ;; single colon is accepted for backward compatibility.
       "\\(?:\\(:" (regexp-opt (append css-pseudo-class-ids
                                       css-pseudo-element-ids)
                               t)
       "\\|::" (regexp-opt css-pseudo-element-ids t) "\\)"
       "\\(?:([^)]+)\\)?"

First we work really hard to see whether we have something that looks
like a selector, i.e. a line that starts with,

p
p::after
p:active

or in the scss case

p#{foo}:active

and then we allow any kind of junk for the rest of the line, like

p.a *[line noise]* p.b {
}

with this bit:

     (if (not sassy)
           "[^:{}()\n]*"
         (concat "[^:{}()\n#]*\\(?:" scss--hash-re "[^:{}()\n#]*\\)*"))
       "\\)*"
       "\\)\\(?:\n[ \t]*\\)*{")
     (1 'css-selector keep))

Anyway, I still don't grok this bit:

         (concat "\\(?:" scss--hash-re
                 "\\|[^@/:{}() \t\n#]\\)"
                 "[^:{}()#]*\\(?:" scss--hash-re "[^:{}()#]*\\)*"))

That is, we're matching an scss/css selector first, and then...
possibly another one?  Which we're doing later, anyway.

So just getting rid of that bit seems to lead to the same highlighting
(and gets rid of the original reported problem).  But like I said, I
don't understand quite what it's trying to do there.  Can anybody come
up with an scss example this breaks?

diff --git a/lisp/textmodes/css-mode.el b/lisp/textmodes/css-mode.el
index 1139fd1976..fd47a7b83b 100644
--- a/lisp/textmodes/css-mode.el
+++ b/lisp/textmodes/css-mode.el
@@ -953,14 +953,13 @@ css--font-lock-keywords
          ;; chars in selectors in the case of #{$foo}
          ;; variable interpolation!
          (concat "\\(?:" scss--hash-re
-                 "\\|[^@/:{}() \t\n#]\\)"
-                 "[^:{}()#]*\\(?:" scss--hash-re "[^:{}()#]*\\)*"))
+                 "\\|[^@/:{}() \t\n][^:{}()]\\)*"))
        ;; Even though pseudo-elements should be prefixed by ::, a
        ;; single colon is accepted for backward compatibility.
        "\\(?:\\(:" (regexp-opt (append css-pseudo-class-ids
                                        css-pseudo-element-ids)
                                t)
-       "\\|::" (regexp-opt css-pseudo-element-ids t) "\\)"
+       "\\|::" (regexp-opt css-pseudo-element-ids t) "\\)?"
        "\\(?:([^)]+)\\)?"
        (if (not sassy)
            "[^:{}()\n]*"

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no