bug#37814: [PATCH] Add an option to preserve ANSI sequences

bug#37814: [PATCH] Add an option to preserve ANSI sequences

[Pablo Barbáchano]

* lisp/ansi-color.el Add an option to preserve the ANSI sequences
* test/lisp/ansi-color-tests.el: Add tests
---
 lisp/ansi-color.el            | 23 +++++++++++-----
 test/lisp/ansi-color-tests.el | 49 +++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 7 deletions(-)
 create mode 100644 test/lisp/ansi-color-tests.el

diff --git a/lisp/ansi-color.el b/lisp/ansi-color.el
index 31bed6028c..e2fb205995 100644
--- a/lisp/ansi-color.el
+++ b/lisp/ansi-color.el
@@ -363,7 +363,7 @@ ansi-color-filter-region
 	  (setq ansi-color-context-region (list nil (match-beginning 0)))
 	(setq ansi-color-context-region nil)))))
 
-(defun ansi-color-apply-on-region (begin end)
+(defun ansi-color-apply-on-region (begin end &optional preserve-sequences)
   "Translates SGR control sequences into overlays or extents.
 Delete all other control sequences without processing them.
 
@@ -380,18 +380,27 @@ ansi-color-apply-on-region
 `ansi-color-apply-on-region'.  Specifically, it will override
 BEGIN, the start of the region and set the face with which to
 start.  Set `ansi-color-context-region' to nil if you don't want
-this."
+this.
+
+If PRESERVE-SEQUENCES is t, the sequences are hidden instead of
+being deleted."
   (let ((codes (car ansi-color-context-region))
-	(start-marker (or (cadr ansi-color-context-region)
-			  (copy-marker begin)))
-	(end-marker (copy-marker end)))
+        (start-marker (or (cadr ansi-color-context-region)
+                          (copy-marker begin)))
+        (end-marker (copy-marker end)))
     (save-excursion
       (goto-char start-marker)
       ;; Find the next escape sequence.
       (while (re-search-forward ansi-color-control-seq-regexp end-marker t)
-        ;; Remove escape sequence.
-        (let ((esc-seq (delete-and-extract-region
+        ;; Extract escape sequence.
+        (let ((esc-seq (buffer-substring
                         (match-beginning 0) (point))))
+          (if preserve-sequences
+              ;; make the escape sequence transparent
+              (overlay-put (make-overlay (match-beginning 0) (point)) 'invisible t)
+            ;; else, strip
+            (delete-region (match-beginning 0) (point)))
+
           ;; Colorize the old block from start to end using old face.
           (funcall ansi-color-apply-face-function
                    (prog1 (marker-position start-marker)
diff --git a/test/lisp/ansi-color-tests.el b/test/lisp/ansi-color-tests.el
new file mode 100644
index 0000000000..8a6cf2e41b
--- /dev/null
+++ b/test/lisp/ansi-color-tests.el
@@ -0,0 +1,49 @@
+;;; ansi-color-tests.el --- Test suite for ansi-color  -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2019 Free Software Foundation, Inc.
+
+;; Author: Pablo Barbáchano <pablo.barbachano@gmail.com>
+;; Keywords: ansi
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;;; Code:
+
+(require 'ansi-color)
+
+(defvar test-strings '(("\e[33mHello World\e[0m" . "Hello World")
+                       ("\e[1m\e[3m\e[5mbold italics blink\e[0m" . "bold italics blink")))
+
+(ert-deftest ansi-color-apply-on-region-test ()
+    (dolist (pair test-strings)
+      (with-temp-buffer
+        (insert (car pair))
+        (ansi-color-apply-on-region (point-min) (point-max))
+        (should (equal (buffer-string) (cdr pair)))
+        (should (not (equal (overlays-at (point-min)) nil))))))
+
+(ert-deftest ansi-color-apply-on-region-preserving-test ()
+    (dolist (pair test-strings)
+      (with-temp-buffer
+        (insert (car pair))
+        (ansi-color-apply-on-region (point-min) (point-max) t)
+        (should (equal (buffer-string) (car pair))))))
+
+(provide 'ansi-color-tests)
+
+;;; ansi-color-tests.el ends here
-- 
2.17.1

bug#37814: [PATCH] Add an option to preserve ANSI sequences

[Eli Zaretskii]

> From: Pablo Barbáchano <pablo.barbachano@gmail.com>
> Date: Fri, 18 Oct 2019 20:18:13 +0200
> Cc: Pablo Barbáchano <pablo.barbachano@gmail.com>
> 
> * lisp/ansi-color.el Add an option to preserve the ANSI sequences
> * test/lisp/ansi-color-tests.el: Add tests

Thank you for your contribution.

Could you please elaborate on the use case(s) that could benefit from
this change?

Also, using overlays would mean that copying the text elsewhere will
reveal the SGR sequences, is that intended?  If not, perhaps using
text properties would be better?

Did you consider using some non-trivial invisibility spec instead of
just t?  It's hard to say if this would make sense without knowing the
use cases you had in mind.

The commit log message is not formatted according to our conventions;
see CONTRIBUTE in the Emacs sources for the details.

Finally, these changes are too large for us to accept them without a
copyright assignment.  Would you like to start the legal paperwork to
that end?  If so, I will send you the form to fill.

Thanks again for your interest in Emacs.

bug#37814: [PATCH] Add an option to preserve ANSI sequences

[Pablo Barbachano]

Hi,

> Could you please elaborate on the use case(s) that could benefit from
> this change?

My use case is when running shell code in org-babel and capturing the resulting output. I can get it to interpret the ANSI sequences, but when I save the org buffer, the ANSI information would get lost.

This is actually what I'm trying to do: https://emacs.stackexchange.com/questions/35364/how-do-i-attach-a-custom-function-to-process-org-mode-babel-shell-output

> Also, using overlays would mean that copying the text elsewhere will
> reveal the SGR sequences, is that intended?  If not, perhaps using
> text properties would be better?

So far I haven't had a need for that. Also, since the rest of the code uses overlays, would it be OK to mix text properties and overlays?

> Did you consider using some non-trivial invisibility spec instead of
> just t?  It's hard to say if this would make sense without knowing the
> use cases you had in mind.

I didn't think of it. But I can see now that a keyword argument would allow for different behaviors:

- nil - delete the sequences (default)
- 'invisible - invisible overlay over the sequences
- 'keep - don't delete the sequences

I will rework the patch to allow that.

> The commit log message is not formatted according to our conventions;
> see CONTRIBUTE in the Emacs sources for the details.

Thanks. I will look at it.

> Finally, these changes are too large for us to accept them without a
> copyright assignment.  Would you like to start the legal paperwork to
> that end?  If so, I will send you the form to fill.

Yes, please, send me the form.

> Thanks again for your interest in Emacs.

Thank you for the fast review!

--
Pablo

bug#37814: [PATCH] Add an option to preserve ANSI sequences

[Eli Zaretskii]

> From: Pablo Barbachano <pablo.barbachano@gmail.com>
> Cc: 37814@debbugs.gnu.org
> Date: Fri, 01 Nov 2019 21:25:28 +0100
> 
> > Finally, these changes are too large for us to accept them without a
> > copyright assignment.  Would you like to start the legal paperwork to
> > that end?  If so, I will send you the form to fill.
> 
> Yes, please, send me the form.

Form sent off-list.

bug#37814: [PATCH] Add an option to preserve ANSI sequences

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Pablo Barbachano <pablo.barbachano@gmail.com>
>> 
>> > Finally, these changes are too large for us to accept them without a
>> > copyright assignment.  Would you like to start the legal paperwork to
>> > that end?  If so, I will send you the form to fill.
>> 
>> Yes, please, send me the form.
>
> Form sent off-list.

This was almost a year ago.

Looking at the copyright assignment list, I can't find Pablo
Barbachano.  Was this abandoned?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#37814: [PATCH] Add an option to preserve ANSI sequences

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Pablo Barbachano <pablo.barbachano@gmail.com>,  37814@debbugs.gnu.org
> Date: Sun, 09 Aug 2020 20:42:14 +0200
> 
> >> Yes, please, send me the form.
> >
> > Form sent off-list.
> 
> This was almost a year ago.
> 
> Looking at the copyright assignment list, I can't find Pablo
> Barbachano.  Was this abandoned?

My records indicate that the FSF copyright clerk sent the printed
assignment to Pablo for signing in Nov 2019, but I have no further
correspondence on that.  Pablo, did you sign and return the paperwork
to the FSF?

bug#37814: [PATCH] Add an option to preserve ANSI sequences

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> My records indicate that the FSF copyright clerk sent the printed
> assignment to Pablo for signing in Nov 2019, but I have no further
> correspondence on that.  Pablo, did you sign and return the paperwork
> to the FSF?

This was five weeks ago, and there was no response, so I'm closing this
bug report.  If progress can be made, please respond to the debbugs mail
address, and we'll reopen the bug report.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no