[Fix #1452] Fix broken ANSI coloring in the repl

[rfkm]

(Temporary?) fix for #1452

[bbatsov]

I'd add more of the explanation of why this is needed here.

[bbatsov]

Apart from my small remarks the code looks OK to me. @Malabarba you're our resident overlays expert - any feedback from you?

[Malabarba]

What does (overlays at -1) yield? If it throws an error then line 490 could be dangerous.

@Malabarba you're our resident overlays expert - any feedback from you?

The overlay code looks solid. But I must say I didn't understand why the modification-hook used by ansi-color isn't enough. Shouldn't it also be triggered whenever insert-behind-hook is triggered?

[rfkm]

@bbatsov Thank you for the review! I'll fix them later.

@Malabarba

What does (overlays at -1) yield? If it throws an error then line 490 could be dangerous.

Just returns nil.

But I must say I didn't understand why the modification-hook used by ansi-color isn't enough. Shouldn't it also be triggered whenever insert-behind-hook is triggered?

I tested the behavior with the following snippet:

(with-temp-buffer
  (insert "foo")
  (let ((ov (make-overlay 1 4)))
    (overlay-put ov 'modification-hooks
                 (list (lambda (ov &rest args)
                         (message "hook %s" args))))
    (message "ov %s" ov)
    (goto-char 4)
    (insert-before-markers "a")
    (message "ov %s" ov)))

In this case, the hook is never called even though the overlay has been extended (tested with 24.4/24.5).

The manual says modification-hooks is called "if any character within the overlay is changed or if text is inserted strictly within the overlay".
I'm not sure what strictly exactly means, but it might mean text is inserted within the range of the overlay which is not extended yet.

Curiously, the comments in ansi-color-make-extent says, "we use the insert-behind-hook'" even though modification-hooks is actually used.
On the other hand, ansi-color-freeze-overlay's docstring says, "This function can be used for the modification-hooks".

[rfkm]

I'm sorry I'm late.
I have just pushed some changes.

The test has been failed, but this seems not to be due to my modification.

[bbatsov]

Seems you'll have to rebase this branch on top of the current master.

[rfkm]

Done

[bbatsov]

@Malabarba are you happy with the current state of the patch?

[Malabarba]

Yes. Sorry for the delay.

[bbatsov]

@rfkm Rebase this and I'll have it merged.

[rfkm]

Rebased. Thanks :-)

[bbatsov]

👍