unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#37814: [PATCH] Add an option to preserve ANSI sequences
@ 2019-10-18 18:18 Pablo Barbáchano
  2019-10-19  6:12 ` Eli Zaretskii
  0 siblings, 1 reply; 7+ messages in thread
From: Pablo Barbáchano @ 2019-10-18 18:18 UTC (permalink / raw)
  To: 37814; +Cc: 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






^ permalink raw reply related	[flat|nested] 7+ messages in thread

* bug#37814: [PATCH] Add an option to preserve ANSI sequences
  2019-10-18 18:18 bug#37814: [PATCH] Add an option to preserve ANSI sequences Pablo Barbáchano
@ 2019-10-19  6:12 ` Eli Zaretskii
  2019-11-01 20:25   ` Pablo Barbachano
  0 siblings, 1 reply; 7+ messages in thread
From: Eli Zaretskii @ 2019-10-19  6:12 UTC (permalink / raw)
  To: Pablo Barbáchano; +Cc: 37814

> 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.





^ permalink raw reply	[flat|nested] 7+ messages in thread

* bug#37814: [PATCH] Add an option to preserve ANSI sequences
  2019-10-19  6:12 ` Eli Zaretskii
@ 2019-11-01 20:25   ` Pablo Barbachano
  2019-11-01 21:01     ` Eli Zaretskii
  0 siblings, 1 reply; 7+ messages in thread
From: Pablo Barbachano @ 2019-11-01 20:25 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 37814


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





^ permalink raw reply	[flat|nested] 7+ messages in thread

* bug#37814: [PATCH] Add an option to preserve ANSI sequences
  2019-11-01 20:25   ` Pablo Barbachano
@ 2019-11-01 21:01     ` Eli Zaretskii
  2020-08-09 18:42       ` Lars Ingebrigtsen
  0 siblings, 1 reply; 7+ messages in thread
From: Eli Zaretskii @ 2019-11-01 21:01 UTC (permalink / raw)
  To: Pablo Barbachano; +Cc: 37814

> 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.





^ permalink raw reply	[flat|nested] 7+ messages in thread

* bug#37814: [PATCH] Add an option to preserve ANSI sequences
  2019-11-01 21:01     ` Eli Zaretskii
@ 2020-08-09 18:42       ` Lars Ingebrigtsen
  2020-08-09 19:38         ` Eli Zaretskii
  0 siblings, 1 reply; 7+ messages in thread
From: Lars Ingebrigtsen @ 2020-08-09 18:42 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Pablo Barbachano, 37814

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





^ permalink raw reply	[flat|nested] 7+ messages in thread

* bug#37814: [PATCH] Add an option to preserve ANSI sequences
  2020-08-09 18:42       ` Lars Ingebrigtsen
@ 2020-08-09 19:38         ` Eli Zaretskii
  2020-09-14 15:04           ` Lars Ingebrigtsen
  0 siblings, 1 reply; 7+ messages in thread
From: Eli Zaretskii @ 2020-08-09 19:38 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: pablo.barbachano, 37814

> 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?





^ permalink raw reply	[flat|nested] 7+ messages in thread

* bug#37814: [PATCH] Add an option to preserve ANSI sequences
  2020-08-09 19:38         ` Eli Zaretskii
@ 2020-09-14 15:04           ` Lars Ingebrigtsen
  0 siblings, 0 replies; 7+ messages in thread
From: Lars Ingebrigtsen @ 2020-09-14 15:04 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 37814, pablo.barbachano

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





^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-09-14 15:04 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-10-18 18:18 bug#37814: [PATCH] Add an option to preserve ANSI sequences Pablo Barbáchano
2019-10-19  6:12 ` Eli Zaretskii
2019-11-01 20:25   ` Pablo Barbachano
2019-11-01 21:01     ` Eli Zaretskii
2020-08-09 18:42       ` Lars Ingebrigtsen
2020-08-09 19:38         ` Eli Zaretskii
2020-09-14 15:04           ` Lars Ingebrigtsen

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).