ANSI coloring in REPL buffer is broken

[rfkm]

How to reproduce

1. Interactively eval the following code:

  (println "foo")
  (println "\u001b[31mbar\u001b[0m")
  (println "baz")

1. Then you will see the following output in REPL buffer:

foo  <-- default face
bar  <-- red face
baz  <-- red face(!)
user>

Cause

This is because new output of interactive-eval extends an overlay for ANSI coloring created before.

The following is a part of output of C-u C-x = at 'r' of 'bar' after evaluating (println "\u001b[31mbar\u001b[0m"):

There is an overlay here:
 From 1 to 4
  face                 (foreground-color . "#fb4934")
  modification-hooks   (ansi-color-freeze-overlay)

Then, the following is new one after evaluating (println "baz"):

There is an overlay here:
 From 1 to 8
  face                 (foreground-color . "#fb4934")
  modification-hooks   (ansi-color-freeze-overlay)

It cannot be probably said that it is due to CIDER because I suspect this is ansi-color's problem.
I think ansi-color-freeze-overlay should prevent overlays from being extended at the first place.
I've actually confirmed it works by modifying ansi-color to use insert-behind-hooks instead of modification-hooks.

However, I think it is not so difficult to introduce a workaround for this on CIDER's side.
For example, it seems to work for me to use insert instead of insert-before-markers here though I'm not sure whether this cause other problems.
(BTW, cider-repl-output-start/cider-repl-output-end seems not to work but it is another issue.)

I'm a novice at elisp (and CIDER internals, of course), so I would be happy if someone investigate this problem further.

[Malabarba]

I think the reason why we use insert-before-markers is to ensure that the input-start marker stays after the inserted output. So that might break if we change that.
However, this whole section of our code looks rather complicated, so maybe it would be good to trim it down a bit. Why do we need so many markers anyway? :-P

[bbatsov]

This code was mostly copy pasted from Slime. I guess it's time to rework
it.

On Saturday, 5 December 2015, Artur Malabarba notifications@github.com
wrote:

I think the reason why we use insert-before-markers is to ensure that the
input-start marker stays after the inserted output. So that might break
if we change that.
However, this whole section of our code looks rather complicated, so maybe
it would be good to trim it down a bit. Why do we need so many markers
anyway? :-P

—
Reply to this email directly or view it on GitHub
#1452 (comment)
.

[rfkm]

I think it does not affect cider-repl-input-start-mark because bol is t only when called via interactive-eval and position is determined by (cider-repl--end-of-line-before-input-start) which is always before cider-repl-input-start-mark.
Yeah, it's not too much to say that it accidentally works even if it's true. :P

Another hacky but more reliable way is manually to add a hook to overlays created by ansi-color-apply-on-region.

(defun cider-repl--emit-interactive-output (string face)
  "Emit STRING as interactive output using FACE."
  (with-current-buffer (cider-current-repl-buffer)
    (let ((pos (cider-repl--end-of-line-before-input-start))
          (string (replace-regexp-in-string "\n\\'" "" string)))
      (cider-repl--emit-output-at-pos (current-buffer) string face pos t)
      (ansi-color-apply-on-region pos (point-max))

      ;; added
      (dolist (ov (overlays-at (1- (cider-repl--end-of-line-before-input-start))))
        (when (member #'ansi-color-freeze-overlay (overlay-get ov 'modification-hooks)) ; ensure ov is crated by ansi-color
          (unless (member #'ansi-color-freeze-overlay (overlay-get ov 'insert-behind-hooks))
            (push #'ansi-color-freeze-overlay (overlay-get ov 'insert-behind-hooks))))))))

[bbatsov]

PR welcome. :-)