GNU bug report logs - #27407
25.2; SGR reset is ignored if it terminates Eshell's output

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 27407 in the body.
You can then email your comments to 27407 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Pierre Neidhardt <ambrevar <at> gmail.com>
To: GNU Emacs bug Tracking System <bug-gnu-emacs <at> gnu.org>
Subject: 25.2; SGR reset is ignored if it terminates Eshell's output
Date: Sat, 17 Jun 2017 12:31:03 +0100

Steps to reproduce:

- M-x eshell
- Enter "hello"

The new prompt becomes yellow.

It does not happen if the SGR reset sequence is not last.
This is fine for instance:

- echo "hello"X
- echo -n "hello"

Maybe an off-by-one error in the `ansi-*' function?



In GNU Emacs 25.2.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.22.10)
 of 2017-04-22 built on juergen
Windowing system distributor 'The X.Org Foundation', version 11.0.11903000
System Description:	Arch Linux

Configured using:
 'configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
 --localstatedir=/var --with-x-toolkit=gtk3 --with-xft --with-modules
 'CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe
 -fstack-protector-strong' CPPFLAGS=-D_FORTIFY_SOURCE=2
 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro'

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GCONF GSETTINGS
NOTIFY ACL GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 MODULES

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Message #8 received at 27407 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Pierre Neidhardt <ambrevar <at> gmail.com>
Cc: 27407 <at> debbugs.gnu.org
Subject: Re: bug#27407: 25.2;
 SGR reset is ignored if it terminates Eshell's output
Date: Sat, 17 Jun 2017 14:19:54 -0400

[Message part 1 (text/plain, inline)]

tags 27407 + patch
severity 27407 minor
quit

Pierre Neidhardt <ambrevar <at> gmail.com> writes:

> Steps to reproduce:
>
> - M-x eshell
> - Enter "hello"
>
> The new prompt becomes yellow.

You meant enter the text produced with (insert "echo \"\e[33mhello\e[0m\"").

> It does not happen if the SGR reset sequence is not last.
> This is fine for instance:
>
> - echo "hello"X
> - echo -n "hello"
>
> Maybe an off-by-one error in the `ansi-*' function?

The escape code is implemented by applying an overlay to the text
following it (see `ansi-color-apply-on-region').  The problem seems to
be that the reset sequence is translated to no face, so nothing is
applied (see `ansi-color-apply-overlay-face'), and the yellow overlay
gets extended despite the `ansi-color-freeze-overlay' modification-hook
because `eshell-output-filter' (and `eshell-send-input') let-bind
`inhibit-modification-hooks'.

I'm not sure why `inhibit-modification-hooks' needs to be let-bound at
all, but just binding it to nil around string insertion seems to be
enough:

[v1-0001-Let-ansi-color-overlay-hooks-work-in-eshell-Bug-2.patch (text/x-diff, inline)]

From 6e3892b8044fc918a5231a79963d0560adea2403 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs <at> gmail.com>
Date: Sat, 17 Jun 2017 12:06:37 -0400
Subject: [PATCH v1] Let ansi-color overlay hooks work in eshell (Bug#27407)

* lisp/ansi-color.el (ansi-color-make-extent): Add
`ansi-color-freeze-overlay' to `insert-behind-hooks' as well.
* lisp/eshell/esh-mode.el (eshell-output-filter): Let-bind
`inhibit-modification-hooks' to nil while inserting the string.
---
 lisp/ansi-color.el      | 1 +
 lisp/eshell/esh-mode.el | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/lisp/ansi-color.el b/lisp/ansi-color.el
index a1b4933175..72d70c2102 100644
--- a/lisp/ansi-color.el
+++ b/lisp/ansi-color.el
@@ -481,6 +481,7 @@ (defun ansi-color-make-extent (from to &optional object)
     ;; property to make sure it works.
     (let ((overlay (make-overlay from to object)))
       (overlay-put overlay 'modification-hooks '(ansi-color-freeze-overlay))
+      (overlay-put overlay 'insert-behind-hooks '(ansi-color-freeze-overlay))
       overlay)))
 
 (defun ansi-color-freeze-overlay (overlay is-after begin end &optional len)
diff --git a/lisp/eshell/esh-mode.el b/lisp/eshell/esh-mode.el
index 0fd0c18301..0999f9c4a8 100644
--- a/lisp/eshell/esh-mode.el
+++ b/lisp/eshell/esh-mode.el
@@ -726,7 +726,9 @@ (defun eshell-output-filter (process string)
 		  (setq obeg (+ obeg nchars)))
 	      (if (<= (point) oend)
 		  (setq oend (+ oend nchars)))
-	      (insert-before-markers string)
+              ;; Let the ansi-color overlay hooks run.
+              (let ((inhibit-modification-hooks nil))
+                (insert-before-markers string))
 	      (if (= (window-start) (point))
 		  (set-window-start (selected-window)
 				    (- (point) nchars)))
-- 
2.11.1

Message #15 received at 27407 <at> debbugs.gnu.org (full text, mbox):

From: Pierre Neidhardt <ambrevar <at> gmail.com>
To: npostavs <at> users.sourceforge.net
Cc: 27407 <at> debbugs.gnu.org
Subject: Re: bug#27407: 25.2; SGR reset is ignored if it terminates Eshell's
 output
Date: Sun, 18 Jun 2017 07:49:17 +0100

On 17-06-17 14:19:54, npostavs <at> users.sourceforge.net wrote:
> You meant enter the text produced with (insert "echo \"\e[33mhello\e[0m\"").

Yes.

I haven't tried your patch, but in the general case Eshell should not extend the
ANSI coloring to the prompt, SGR reset or not. I.e. the following should not
color the prompt:

	(insert "echo \"\e[33mhello")

--
Pierre Neidhardt

Message #18 received at 27407 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Pierre Neidhardt <ambrevar <at> gmail.com>
Cc: 27407 <at> debbugs.gnu.org
Subject: Re: bug#27407: 25.2;
 SGR reset is ignored if it terminates Eshell's output
Date: Sun, 18 Jun 2017 13:50:20 -0400

Pierre Neidhardt <ambrevar <at> gmail.com> writes:

> On 17-06-17 14:19:54, npostavs <at> users.sourceforge.net wrote:
>
> I haven't tried your patch, but in the general case Eshell should not extend the
> ANSI coloring to the prompt, SGR reset or not. I.e. the following should not
> color the prompt:
>
> 	(insert "echo \"\e[33mhello")

My patch does not solve this scenario, but on the other hand, when I try
this experiment in a terminal with bash, not putting a reset does give a
coloured prompt, i.e., in the following the 2nd "~/src$" is in yellow:

    ~/src$ echo $'\e[33mhello'
    hello
    ~/src$ 

So should eshell really be any different?

Message #21 received at 27407 <at> debbugs.gnu.org (full text, mbox):

From: Pierre Neidhardt <ambrevar <at> gmail.com>
To: npostavs <at> users.sourceforge.net
Cc: 27407 <at> debbugs.gnu.org
Subject: Re: bug#27407: 25.2; SGR reset is ignored if it terminates Eshell's
 output
Date: Sun, 18 Jun 2017 19:07:42 +0100

> My patch does not solve this scenario, but on the other hand, when I try
> this experiment in a terminal with bash, not putting a reset does give a
> coloured prompt, i.e., in the following the 2nd "~/src$" is in yellow:
>
>     ~/src$ echo $'\e[33mhello'
>     hello
>     ~/src$
>
> So should eshell really be any different?

Yes. Definitely. Eshell _already_ is very different and that's why we use
it. Bash is hardly a role model in the world of shells.

In the case of bash, the issue probably stems back from historical limitations
of VT* terminals. Something that does not make sense to emulate in modern
computers.

I don't see any use case for letting the SGR color the shell. If you want to
change you shell from a command, Elisp is your friend...

--
Pierre Neidhardt

Message #24 received at 27407 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Pierre Neidhardt <ambrevar <at> gmail.com>
Cc: 27407 <at> debbugs.gnu.org
Subject: Re: bug#27407: 25.2;
 SGR reset is ignored if it terminates Eshell's output
Date: Sun, 18 Jun 2017 15:34:11 -0400

[Message part 1 (text/plain, inline)]

Pierre Neidhardt <ambrevar <at> gmail.com> writes:

>> My patch does not solve this scenario, but on the other hand, when I try
>> this experiment in a terminal with bash, not putting a reset does give a
>> coloured prompt, i.e., in the following the 2nd "~/src$" is in yellow:
>>
>>     ~/src$ echo $'\e[33mhello'
>>     hello
>>     ~/src$
>>
>> So should eshell really be any different?
>
> Yes. Definitely. Eshell _already_ is very different and that's why we use
> it. Bash is hardly a role model in the world of shells.
>
> In the case of bash, the issue probably stems back from historical limitations
> of VT* terminals. Something that does not make sense to emulate in modern
> computers.
>
> I don't see any use case for letting the SGR color the shell. If you want to
> change you shell from a command, Elisp is your friend...

Hmm, seems easy enough to fix:

[v1-0001-Reset-ansi-escape-context-before-printing-eshell-.patch (text/x-diff, inline)]

From 34f41c8573936285076b4e24c2e7ff4d34a48b8d Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs <at> gmail.com>
Date: Sun, 18 Jun 2017 15:29:55 -0400
Subject: [PATCH v1] Reset ansi escape context before printing eshell prompt
 (Bug#27407)

* lisp/eshell/em-prompt.el (eshell-emit-prompt): Reset
`ansi-color-context-region'.
---
 lisp/eshell/em-prompt.el | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lisp/eshell/em-prompt.el b/lisp/eshell/em-prompt.el
index 53a83e6a67..8c81b43b1f 100644
--- a/lisp/eshell/em-prompt.el
+++ b/lisp/eshell/em-prompt.el
@@ -117,6 +117,8 @@ (defun eshell-prompt-initialize ()
 
 (defun eshell-emit-prompt ()
   "Emit a prompt if eshell is being used interactively."
+  (when (boundp 'ansi-color-context-region)
+    (setq ansi-color-context-region nil))
   (run-hooks 'eshell-before-prompt-hook)
   (if (not eshell-prompt-function)
       (set-marker eshell-last-output-end (point))
-- 
2.11.1

Message #27 received at 27407 <at> debbugs.gnu.org (full text, mbox):

From: npostavs <at> users.sourceforge.net
To: Pierre Neidhardt <ambrevar <at> gmail.com>
Cc: 27407 <at> debbugs.gnu.org
Subject: Re: bug#27407: 25.2;
 SGR reset is ignored if it terminates Eshell's output
Date: Mon, 03 Jul 2017 10:12:26 -0400

tags 27407 fixed
close 27407 26.1
quit

npostavs <at> users.sourceforge.net writes:

>>
>> I don't see any use case for letting the SGR color the shell. If you want to
>> change you shell from a command, Elisp is your friend...
>
> Hmm, seems easy enough to fix:

Pushed to master.

[1: 4ebdcc46ab]: 2017-07-03 10:09:40 -0400
  Let ansi-color overlay hooks work in eshell (Bug#27407)
  http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=4ebdcc46ab345849332332d580bd1e3c2c9adb1e

[2: 37cdfec7c7]: 2017-07-03 10:09:41 -0400
  Reset ansi escape context before printing eshell prompt (Bug#27407)
  http://git.savannah.gnu.org/cgit/emacs.git/commit/?id=37cdfec7c73205668498da9b27387f5f3ccfebee

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.