joaotavora / sly

Sylvester the Cat's Common Lisp IDE
1.27k stars 145 forks source link

Add optional support for ANSI colors #332

Closed Ambrevar closed 4 years ago

Ambrevar commented 4 years ago

One recurring annoyance is using prove with SLY: by default, it tries to use ANSI colors, which results in a lot of garbage being displayed in SLY:


× 1 of 2 tests failed (116ms)

Prove is an archived project, so it won't receive any update to fix this: https://github.com/fukamachi/prove#colorize-test-reports-on-slime

I found out that Emacs offers an easy solution:

  (defun sly-colorize-buffer (str)
      (ansi-color-apply str))
  (add-hook 'sly-mrepl-output-filter-functions 'sly-colorize-buffer)

What about adding this by default, or maybe as an option?

PuercoPop commented 4 years ago

@Ambrevar You don't need any support from Sly. By virtue of using comint mode you can use xterm-color to replace the characters with the appropriate colors.

Before I learned about xterm-color, with the help of Joao, I ported the SLIME contrib to SLY. It is a contrib that basic does what you are suggesting: https://github.com/PuercoPop/sly-repl-ansi-color

joaotavora commented 4 years ago

@PuercoPop, @Ambrevar isn't it much easier to:

(add-hook 'sly-mrepl-mode-hook 
  (lambda ()
    (add-hook 'comint-preoutput-filter-functions 'ansi-color-apply nil t)))

Instead of messing with compilation and so on and so forth?

(EDIT: I'm responding to a solution you since deleted)

PuercoPop commented 4 years ago

(EDIT: I'm responding to a solution you since deleted)

Yeah sorry, I deleted because I realized most of it was unrelated to comint (just woke up). The relevant part was (add-hook 'comint-preoutput-filter-functions 'xterm-color-filter t) which is similar to what you are suggesting.

joaotavora commented 4 years ago

eah sorry, I deleted because I realized most of it was unrelated to comint (just woke up). The relevant part was (add-hook 'comint-preoutput-filter-functions 'xterm-color-filter t) which is similar to what you are suggesting.

No probblem. I also realized I made a mistake. The nil t final args to add-hook are essencial so that the hook is only in effect in the mrepl buffer.

Ambrevar commented 4 years ago

@joaotavora Yes, I think your solution is ideal. @PuercoPop Is there a particular benefit in using the Xterm package for this?

Ambrevar commented 4 years ago

@joaotavora However, you should define the function, lambdas are frown upon in hooks as they are blackboxes that are hard to remove after the fact.

joaotavora commented 4 years ago

@joaotavora However, you should define the function, lambdas are frown upon in hooks as they are blackboxes that are hard to remove after the fact.

Yes, very true.

PuercoPop commented 4 years ago

@Ambrevar xterm-color supports more colors (xterm-256). Although for rove it doesn't matter

Ambrevar commented 4 years ago

@joaotavora Did you document this or add an option somewhere?

joaotavora commented 4 years ago

@joaotavora Did you document this or add an option somewhere?

No. I got the impression that you were happy with the add-hook solution. You think something should be added to the manual?

Ambrevar commented 4 years ago

Yes, I think it'd be nice to have the code snippet you shared in the manual (with a named function instead of the lambda).

Thanks!