jlr / rainbow-delimiters

Emacs rainbow delimiters mode
http://www.emacswiki.org/emacs/RainbowDelimiters
112 stars 12 forks source link

Don't colour comment delimiters #32

Closed ivan-m closed 10 years ago

ivan-m commented 10 years ago

It seems as of the latest release (obtained via MELPA) that - at least in haskell-mode - that the braces in block comments {- ... -} are being coloured by rainbow-delimiters rather than being coloured as comments.

Specifically, doing describe-face says that both font-lock-comment-delimiter-face and rainbow-delimiters-depth-1-face are being applied (with the latter overriding the former).

purcell commented 10 years ago

Yes, I can confirm this. I'm sure @Fanael will have a clue about what's going on... :-)

Fanael commented 10 years ago

Of course I have. Doesn't mean I have any idea how to fix it.

Part of the problem is rainbow-delimiters-make-syntax-table, where we create our own syntax table, overwriting the major mode entries for delimiters. As it happens, in haskell-mode it means that the innards of rainbow-delimiters no longer recognize { and } as possible block comment delimiters.

Consider this code:

{- This is a completely useless function (yes, really). -}
foo :: Int -> Int
foo x = (x + 5) * 2

Currently, the following delimiters are highlighted:

{- This is a completely useless function (yes, really). -}
^                                        ^           ^   ^

It's a bit unexpected, but at least it's consistent.

Now, let's see what happens if the definition of rainbow-delimiters-make-syntax-table is turned into the identity function.

(progn
  (fset #'rainbow-delimiters-make-syntax-table #'identity)
  (rainbow-delimiters-mode))

Now, the highlighted delimiters are:

{- This is a completely useless function (yes, really). -}
^

What?

rainbow-delimiters-char-ineligible-p doesn't catch the opening brace, because rainbow-delimiters-syntax-ppss does not recognize it as a part of a string nor a part of a comment. It's not a problem with rainbow-delimiters-syntax-ppss, because the same happens if I call the standard syntax-ppss on it, even in a brand new buffer where rainbow-delimiters-mode was never enabled. Try it for yourself: find a block comment, place the point on the opening brace, do M-: (syntax-ppss) RET, and observe that the fourth (are we in a string?) and fifth (are we in a comment?) positions are nil.

Will investigate further later.

Fanael commented 10 years ago

b3f87cfe999ff745185185dae7057b2de102aeae is the first bad commit.

Fanael commented 10 years ago

The actual cause of this bug exists even in eb7b957adc and it can easily be triggered there by changing font-lock-face to face, b3f87cf merely exposed it.

purcell commented 10 years ago

Of course I have. Doesn't mean I have any idea how to fix it.

:-)

I'm a committer on haskell-mode, so if you think that the upstream code is doing something wrong, please let me know. It looks vaguely reasonable to me.

purcell commented 10 years ago

If you move the point | from {|- to {-|, then syntax-ppss returns 1 in the comment slot. I think this is probably the intended behaviour of syntax-ppss: it claims to reflect the parser state starting from the beginning of the buffer, so after { it's not yet sure it's in a comment.

Fanael commented 10 years ago

I'm a committer on haskell-mode, so it you think that the upstream code is doing something wrong, please let me know.

I'm pretty sure it's not an upstream issue.

I think this is probably the intended behaviour of syntax-ppss: it reflects the parser state starting from the beginning of the buffer, so after { it's not yet sure it's in a comment.

Right, that's correct.

Fanael commented 10 years ago

I think I have a patch. Can you try it and see if it doesn't break anything?

 rainbow-delimiters.el | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/rainbow-delimiters.el b/rainbow-delimiters.el
index 72ae2ba..5785f79 100644
--- a/rainbow-delimiters.el
+++ b/rainbow-delimiters.el
@@ -362,12 +362,18 @@ The syntax table is constructed by the function
 (defun rainbow-delimiters-make-syntax-table (syntax-table)
   "Inherit SYNTAX-TABLE and add delimiters intended to be highlighted by mode."
   (let ((table (copy-syntax-table syntax-table)))
-    (modify-syntax-entry ?\( "()  " table)
-    (modify-syntax-entry ?\) ")(  " table)
-    (modify-syntax-entry ?\[ "(]" table)
-    (modify-syntax-entry ?\] ")[" table)
-    (modify-syntax-entry ?\{ "(}" table)
-    (modify-syntax-entry ?\} "){" table)
+    (when (/= ?\( (char-syntax ?\())
+      (modify-syntax-entry ?\( "()" table))
+    (when (/= ?\( (char-syntax ?\[))
+      (modify-syntax-entry ?\[ "(]" table))
+    (when (/= ?\( (char-syntax ?\{))
+      (modify-syntax-entry ?\{ "(}" table))
+    (when (/= ?\) (char-syntax ?\)))
+      (modify-syntax-entry ?\) ")(" table))
+    (when (/= ?\) (char-syntax ?\]))
+      (modify-syntax-entry ?\) ")[" table))
+    (when (/= ?\) (char-syntax ?\}))
+      (modify-syntax-entry ?\) "){" table))
     table))

 (defsubst rainbow-delimiters-depth (loc)
@@ -430,6 +436,10 @@ Returns t if char at loc meets one of the following conditions:
   (or
    (nth 3 ppss)                ; inside string?
    (nth 4 ppss)                ; inside comment?
+   (save-excursion             ; starting a comment?
+     (backward-char)
+     (let ((inhibit-changing-match-data t))
+       (looking-at comment-start-skip)))
    (and rainbow-delimiters-escaped-char-predicate
         (funcall rainbow-delimiters-escaped-char-predicate loc))))
purcell commented 10 years ago

That seems to work nicely here.

Fanael commented 10 years ago

This change breaks fasm-mode, but that's because that major mode was written by a total Emacs noob who just copied code chunks hoping they'll somehow work when put together.

purcell commented 10 years ago

was written by a total Emacs noob who just copied code chunks hoping they'll somehow work when put together

We've all been there at one stage or another... :-)