Edge case causing incorrect color selection

[purcell]

In the snippet below, which is a valid and balanced elisp sexp, the final closing paren is highlighted in red, whereas the opening paren is highlighted in white:

(defun haml-highlight-interpolation (limit)
  "Highlight Ruby interpolation (#{foo}).
LIMIT works as it does in `re-search-forward'."
  (when (re-search-forward "\\(#{\\)" limit t)
    (save-match-data
      (forward-char -1)
      (let ((beg (point)))
        (haml-limited-forward-sexp limit)
        (haml-fontify-region-as-ruby (+ 1 beg) (point)))

      (when (eq (char-before) ?})
        (put-text-property (- (point) 1) (point)
                           'face font-lock-variable-name-face))
      t)))

It looks like the syntax table tweaks in rainbow-delimiters-syntax-table are subtly wrong, because with the point just before the trailing paren:

(car (syntax-ppss (point))) => 1

but

(with-syntax-table rainbow-delimiters-syntax-table
  (car (syntax-ppss (point)))) => 0

Sorry that I'm not immediately sure how best to debug this.

I should add that this is with Emacs HEAD.

[purcell]

Okay, now I see the corresponding note:

;; NOTE: standard char read syntax '?)' is not tested for because emacs manual
;; states punctuation such as delimiters should _always_ use escaped '?\)' form.

So the source isn't strictly corresponding to the elisp norms, and should probably be fixed. But would there be any harm in also checking for the ?) forms? Indeed, the following adjusted version of rainbow-delimiters-char-ineligible-p seems to work correctly:

(defsubst rainbow-delimiters-char-ineligible-p (loc)
  "Return t if char at LOC should be skipped, e.g. if inside a comment.

Returns t if char at loc meets one of the following conditions:
- Inside a string.
- Inside a comment.
- Is an escaped char, e.g. ?\)"
  (let ((parse-state (syntax-ppss loc)))
    (or
     (nth 3 parse-state)                ; inside string?
     (nth 4 parse-state)                ; inside comment?
     (eq (char-before loc) ?\?) ; deprecated ?) escaped char syntax
     (and (eq (char-before loc) ?\\)  ; escaped char, e.g. ?\) - not counted
          (and (not (eq (char-before (1- loc)) ?\\)) ; special-case: ignore ?\\
               (eq (char-before (1- loc)) ?\?))))))

[jlr]

You're right. It wasn't reasonable to expect everyone to remember this spec detail; I think I had wanted to keep the code a line simpler. I'll add in your change. Thanks!

[purcell]

It looks like the original code was changed recently anyway (in 73b15f1b77d0) -- perhaps the above case works by default now.

[jlr]

Actually I'm in the middle of trying to get the changes sorted out here - I merged the commit you reference, and I am having a lot of trouble getting it to ignore ?) now the way you showed. I'm not sure what I'm missing here.

[purcell]

Yeah, true -- the resulting code in rainbow-delimiters-escaped-char-predicate-lisp isn't at all equivalent to what was in the (and ...) expression previously.

[jlr]

At this point I am not sure. I hope I can sort it out soon but can't promise; I'm doing what I think should be working. It's an important bug - other people have written about it often bothering them with complex elisp code. Thank you very much for alerting me to it and I'm sorry I haven't been on top of maintaining this mode for a while.

[purcell]

No problem. I'll take a quick look at the code and see if I can suggest a fix.

[purcell]

Hmm, odd indeed. With the following version of the elisp predicate function, rainbow-delimiters-char-ineligible-p correctly returns t on the brace in ?}, but the color sequence still gets messed up.

(defun rainbow-delimiters-escaped-char-predicate-emacs-lisp (loc)
  (or (eq (char-before loc) ?\?) ; deprecated ?) escaped char syntax)
      (and (eq (char-before loc) ?\\)  ; escaped char, e.g. ?\) - not counted
           (and (not (eq (char-before (1- loc)) ?\\)) ; special-case: ignore ?\\
                (eq (char-before (1- loc)) ?\?)))))

[purcell]

Hmm, okay. The problem actually seems to be related to the opening brace in the "\\(#{\\)" string literal. Not sure what's happening there, but if that is removed, the parens get colored correctly.

[purcell]

In fact, similarly, if the { in the docstring of the sample failure case is removed, the parens also get colored correctly.

[jlr]

Thank you for that investigation; I would not have caught that readily at all and it gives me a place to start from. I hope to correct it as soon as I can. I want this mode to work perfectly!

[purcell]

I confess that I investigated this problem for quite a while yesterday, but I couldn't figure out what was wrong. It must be very subtle, but hey, that makes it interesting. ;-)

[jlr]

Thanks so much for the work on it. I'll look at it too. I'm going to start by looking at anything fundamental about the handling of escaped characters that might cause the code not to work. For now, I might revert to where your change worked, and try to integrate the wonderful code that was sent in for language-specific escape character choices manually, in order to find a path that makes it work.