Ignore blacklisted delimiters altogether.

[Fanael]

Blacklisted delimiters don't contribute to depth anymore.

This was suggested by a user some time ago.

In theory, ignoring blacklisted delimiters is a very simple operation. You just need to modify the syntax table so they're not delimiters, so syntax-ppss doesn't count them when calculating depth.

Okay, I lied. It's not that simple. You see, the buffer-local syntax table is not the only place syntax table entries can come from. Text properties and overlays are cromulent sources of syntax table entries, too. Even ignoring the, admittedly ridiculous, idea that the major mode and other minor modes are messing with us by changing text properties on random characters, it's much harder than modifying the syntax table. Besides, we only just stopped modifying the syntax table.

Let's assume we've dealt with syntax entries somehow. We're done now, aren't we? After all, determining the depth is just a (syntax-ppss) call away, right? No. syntax-ppss is reasonably fast because it uses a cache internally. If you change the syntax in any way without telling syntax-ppss about it, it will use its cache, and that cache will contain stale entries from before the syntax change.

Okay, so we can't use syntax-ppss without telling it to drop the caches. So maybe, I don't know, tell it to drop the caches? (syntax-ppss-flush-cache 0) is not exactly rocket science, is it? As it happens, there's exactly one call to syntax-ppss, so dropping the caches not only will turn that call into (parse-partial-sexp (point-min) pos), it also will slow down all the code calling (syntax-ppss), because after rainbow-delimiters--propertize is done, the cache has to be dropped again, pretty much turning all the calls to syntax-ppss into parse-partial-sexp from (point-min).

If that's the case, would building our own private cache around parse-partial-sexp be a good idea? Maybe. But it duplicates existing code, and there's really nothing wrong with syntax-ppss. Besides, we only just stopped doing that, too.

So, what does this patch do?

I feel filthy having created this atrocity. This patch is made 100% of completely irredeemable pure evil. And it's not the nice kind of evil that turns Emacs into a decent editor. No, it's the evil that murders puppies just for the hell of it. Why, you ask?

• It hooks into syntax-propertize-function and calls the original function. Why is this chaining bad? This comment, found right in the middle of the syntax-propertize-function definition, explains it well:

;; Rather than a -functions hook, this is a -function because it's easier
;; to do a single scan than several scans: with multiple scans, one cannot
;; assume that the text before point has been propertized, so syntax-ppss
;; gives unreliable results (and stores them in its cache to boot, so we'd
;; have to flush that cache between each function, and we couldn't use
;; syntax-ppss-flush-cache since that would not only flush the cache but also
;; reset syntax-propertize--done which should not be done in this case).

• It uses the category text property. Never heard of it? I'm not surprised, me too, until when I saw cc-mode using it when I was writing the rainbow-delimiters test suite. category is a text property indirection. When you put a category on some text, you can affect that text's effective properties by changing the category's symbol plist. Spooky action at a distance? You bet.
• It cracks open the skull of syntax-ppss, removes its brain, replaces it with a different brain, does some work and then gives syntax-ppss its old brain again. That's the entire point of rainbow-delimiters--with-syntax-ppss-cache. It lets us use syntax-ppss without worrying about its cache: the one rainbow-delimiters sees is separate from the one seen by everyone else.
• It doesn't clean up after itself very well. Even if you change the major mode, the categories are still there. Clearly something more dynamic than syntax-propertize-function is needed? There's a thing dynamic just enough: font-lock-mode. I shudder at the thought of a font-lock entry, and one run late at that, and one managed by a major-mode-oblivious minor mode on top of that, managing syntactic information.

The underlying idea, though, is pretty simple. font-lock-mode always ensures the syntactic properties are set before highlighting, so we register a syntax-propertize-function that finds all eligible blacklisted delimiters and puts a, inert at the moment, category on them. Later, inside rainbow-delimiters--propertize, we temporarily modify the syntax codes (reverting it when rainbow-delimiters--propertize is done with its job) by setting the category's syntax-table to '(1). As per (elisp)Syntax Table Internals., that's punctuation. syntax-ppss sees this and treats the would-be delimiter as any other punctuation.

The idea is okay. The implementation I'm utterly disgusted with.

@purcell: where do I turn in my Emacs Lisp hacker badge?

As it is, this patch and being mergeable are not even in the same galaxy. It's a quickly hacked together proof of concept. It may be a decent starting point, though.

[purcell]

I feel filthy having created this atrocity. This patch is made 100% of completely irredeemable pure evil.

This cracked me up. I'm not sure if this PR is a work of subversive software wizardry or dark comedy.

[Fanael]

I'm not sure if this PR is a work of subversive software wizardry or dark comedy.

Probably both.

[Fanael]

@purcell: do you think it's worth keeping the special case of not highlighting Emacs Lisp ?)? This syntax is discouraged, many other modes such as show-paren-mode or hl-sexp get confused by it, the current way we handle it solves one part of the problem, but causes a different problem, and I really think the onus should be on the major mode to set up the syntax table entries properly.

[purcell]

I'm probably the wrong person to ask, because IIRC I was the one who complained about that special case not being handled. At this stage I guess I don't care too much, but it would be a shame if parens got wrongly highlighted as mismatched in a source file which contains ?) or ?(.

[Fanael]

it would be a shame if parens got wrongly highlighted as mismatched in a source file which contains ?) or ?(

They already are, except in a different way. If you want it to stay, I guess this code can stay. In fact, I have an idea how to make it slightly less dumb.

Can you review the PR, in case I'm doing something stupid?

[Fanael]

it would be a shame if parens got wrongly highlighted as mismatched in a source file which contains ?) or ?(

I just found out that unescaped brackets can even break indentation:

(pcase foo
  (?(
     (bar-1))
    (?)
    (bar-2))
  (?[
     (bar-3))
;;;         ^
;;; The above matches [ in the current master, and thus is highlighted as
;;; mismatched, despite our efforts to handle this situation gracefully.
;;; OTOH, EVIL-ignore-blacklisted-delims-altogether gets it right.
    (?]
    (bar-4)))

(pcase foo
  (?\(
   (bar-1))
  (?\)
   (bar-2))
  (?\[
   (bar-3))
  (?\]
   (bar-4)))

Frankly, now I think that any code containing ?( or other unquoted brackets should be fixed ASAP.

[purcell]

Frankly, now I think that any code containing ?( or other unquoted brackets should be fixed ASAP.

Sounds like a reasonable position to take.

Can you review the PR, in case I'm doing something stupid?

Looks okay to me, but you're the expert!

[Fanael]

Looks okay to me, but you're the expert!

Am I? I think that's pretty far from truth.

[Fanael]

I mean, seriously, I can't even get the test suite to pass.

[purcell]

I mean, seriously, I can't even get the test suite to pass.

Ah, see, if you hadn't added tests, you wouldn't have that problem, and you'd still look like an expert. ;-)

[Fanael]

Ah, see, if you hadn't added tests, you wouldn't have that problem

Sometimes I think this is the only reason most projects pass their tests.

[purcell]

Software is a cruel master.

[Fanael]

…and I realized there's no before-change-function to flush rainbow-delimiters--syntax-ppss-cache.

Besides, the code required to pass syntax-table-entries-dont-work-when-blacklisted will be incredibly nasty, even compared to the mess this branch already introduces. I'm thinking this branch is a no go.