GNU bug report logs - #65604
[PATCH] Display the exit code if the last command failed in Eshell

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 65604 in the body.
You can then email your comments to 65604 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: Davide Masserut <dm <at> mssdvd.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Display the exit code if the last command failed in Eshell
Date: Wed, 30 Aug 2023 00:24:28 +0200

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

Tags: patch

There are commands that fail without printing any messages, but 
set specific error codes.

This patch extends the default prompt function to show the exit 
code of the previous failed command.

Before:

~ $ false
~ $

After:

~ $ false
~ [1] $

I believe this is a good default, since it is displayed only when 
a error occurs and hopefully makes debugging easier by showing the 
error code without further input.


In GNU Emacs 30.0.50 (build 10, x86_64-pc-linux-gnu, GTK+ Version
3.24.38, cairo version 1.17.8) of 2023-08-29 built on T480s
Repository revision: ed77dc17f657d99ccf23778c14f06f7226f478f0
Repository branch: master
System Description: Arch Linux

Configured using:
'configure -C --prefix /home/davide/.local --with-pgtk
--with-native-compilation --enable-link-time-optimization
--enable-locallisppath=/usr/share/emacs/site-lisp/
'CFLAGS=-march=native -O2''

[0001-Display-the-exit-code-if-the-last-command-failed-in-.patch (text/patch, attachment)]

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Davide Masserut <dm <at> mssdvd.com>, 65604 <at> debbugs.gnu.org
Subject: Re: bug#65604: [PATCH] Display the exit code if the last command
 failed in Eshell
Date: Tue, 29 Aug 2023 18:52:40 -0700

On 8/29/2023 3:24 PM, Davide Masserut via Bug reports for GNU Emacs, the 
Swiss army knife of text editors wrote:
> Tags: patch
> 
> There are commands that fail without printing any messages, but set 
> specific error codes.
> 
> This patch extends the default prompt function to show the exit code of 
> the previous failed command.

Thanks. I think this makes sense as an option, but I wonder if this is 
the right default place to put it. Instead, what about putting the exit 
status in the mode-line, like with compilation buffers? Eshell already 
uses the mode-line to show when a command is running, so I think it's an 
obvious enhancement to show the status of a command that just finished 
running. This does mean you don't see the *history* of failed commands, 
but it still provides useful feedback for users without requiring a 
change to the prompt (which is a bit more in-your-face).

In the future, I hope to make it easier for people to customize the 
Eshell prompt without writing (as much) Elisp, e.g. by defining it the 
same way we can define the mode-line. However, I haven't finished up 
that patch yet, so it's sitting on one of my pile of local branches...

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

From: Davide Masserut <dm <at> mssdvd.com>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 65604 <at> debbugs.gnu.org
Subject: Re: bug#65604: [PATCH] Display the exit code if the last command
 failed in Eshell
Date: Wed, 30 Aug 2023 11:18:49 +0200

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

Jim Porter <jporterbugs <at> gmail.com> writes:

> Thanks. I think this makes sense as an option, but I wonder if 
> this is
> the right default place to put it. Instead, what about putting 
> the
> exit status in the mode-line, like with compilation buffers? 
> Eshell
> already uses the mode-line to show when a command is running, so 
> I
> think it's an obvious enhancement to show the status of a 
> command that
> just finished running. This does mean you don't see the 
> *history* of
> failed commands, but it still provides useful feedback for users
> without requiring a change to the prompt (which is a bit more
> in-your-face).

I have made the changes you suggested.

[0001-Display-the-exit-code-if-the-last-command-failed-in-.patch (text/x-patch, inline)]

From 8b9f3870e00cdf920e803d92138a9bb0f3c3b645 Mon Sep 17 00:00:00 2001
From: Davide Masserut <dm <at> mssdvd.com>
Date: Wed, 30 Aug 2023 16:38:07 +0200
Subject: [PATCH] Display the exit code if the last command failed in Eshell

* etc/NEWS: Announce change.
* lisp/eshell/esh-cmd.el (eshell-exec-lisp):
(eshell-lisp-command): Use new helper function.
* lisp/eshell/esh-io.el:
(eshell-close-handles): Use new helper function.
(eshell-update-last-command-status): Add new helper function.
* test/lisp/eshell/em-io-tests.el
(em-io-test/modeline-after-failure): Add new test.
---
 etc/NEWS                         |  3 +++
 lisp/eshell/esh-cmd.el           |  8 ++++----
 lisp/eshell/esh-io.el            | 15 ++++++++++++++-
 test/lisp/eshell/esh-io-tests.el | 15 +++++++++++++++
 4 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 9a98db8c83a..810172e3b11 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -370,6 +370,9 @@ to load the edited aliases.
 Running 'rgrep' in Eshell now uses the Emacs grep facility instead of
 calling external rgrep.
 
+---
+*** If the last command failed, its exit code is now displayed in the modeline.
+
 ** Pcomplete
 
 ---
diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index 80066263396..3672481a66a 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -1407,10 +1407,10 @@ eshell-exec-lisp
      ;; command status to some non-zero value to indicate an error; to
      ;; match GNU/Linux, we use 141, which the numeric value of
      ;; SIGPIPE on GNU/Linux (13) with the high bit (2^7) set.
-     (setq eshell-last-command-status 141)
+     (eshell-update-last-command-status 141)
      nil)
     (error
-     (setq eshell-last-command-status 1)
+     (eshell-update-last-command-status 1)
      (let ((msg (error-message-string err)))
        (if (and (not form-p)
                 (string-match "^Wrong number of arguments" msg)
@@ -1481,8 +1481,8 @@ eshell-lisp-command
   (unless eshell-allow-commands
     (signal 'eshell-commands-forbidden '(lisp)))
   (catch 'eshell-external               ; deferred to an external command
-    (setq eshell-last-command-status 0
-          eshell-last-arguments args)
+    (eshell-update-last-command-status 0)
+    (setq eshell-last-arguments args)
     (let* ((eshell-ensure-newline-p (eshell-interactive-output-p))
            (command-form-p (functionp object))
            (result
diff --git a/lisp/eshell/esh-io.el b/lisp/eshell/esh-io.el
index c07f871dd37..d734a83e02f 100644
--- a/lisp/eshell/esh-io.el
+++ b/lisp/eshell/esh-io.el
@@ -70,6 +70,7 @@
 
 (require 'esh-arg)
 (require 'esh-util)
+(require 'compile)
 
 (eval-when-compile
   (require 'cl-lib))
@@ -362,7 +363,7 @@ eshell-close-handles
 RESULT is the quoted value of the last command.  If nil, then use
 the value already set in `eshell-last-command-result'."
   (when exit-code
-    (setq eshell-last-command-status exit-code))
+    (eshell-update-last-command-status exit-code))
   (when result
     (cl-assert (eq (car result) 'quote))
     (setq eshell-last-command-result (cadr result)))
@@ -670,5 +671,17 @@ eshell-output-object
     (dolist (target targets)
       (eshell-output-object-to-target object target))))
 
+(defun eshell-update-last-command-status (exit-code)
+  "Set `eshell-last-command-status' to EXIT-CODE and update `mode-line-process'."
+  (setq mode-line-process
+        (when (> exit-code 0)
+            (list
+             (let ((out-string (format ":[%s]" exit-code))
+                   (msg (format "Last command exited with code %s" exit-code)))
+               (propertize out-string
+                           'help-echo msg
+                           'face 'compilation-mode-line-fail))))
+        eshell-last-command-status exit-code))
+
 (provide 'esh-io)
 ;;; esh-io.el ends here
diff --git a/test/lisp/eshell/esh-io-tests.el b/test/lisp/eshell/esh-io-tests.el
index ce80f3a8f08..c134f262007 100644
--- a/test/lisp/eshell/esh-io-tests.el
+++ b/test/lisp/eshell/esh-io-tests.el
@@ -23,6 +23,7 @@
 (require 'ert-x)
 (require 'esh-mode)
 (require 'eshell)
+(require 'compile)
 
 (require 'eshell-tests-helpers
          (expand-file-name "eshell-tests-helpers"
@@ -370,4 +371,18 @@ esh-io-test/virtual/dev-kill
    (eshell-insert-command "echo three >> /dev/kill")
    (should (equal (car kill-ring) "twothree"))))
 
+(ert-deftest esh-io-test/modeline-after-failure ()
+  "Check that exit code is displayed after a failure."
+  (with-temp-eshell
+   (let ((debug-on-error nil))
+     (eshell-insert-command "(zerop \"foo\")")) ; A failed command.
+   (should (equal-including-properties
+            mode-line-process
+            (list
+             (let ((out-string (format ":[%s]" eshell-last-command-status))
+                   (msg (format "Last command exited with code %s" eshell-last-command-status)))
+               (propertize out-string
+                           'help-echo msg
+                           'face 'compilation-mode-line-fail)))))))
+
 ;;; esh-io-tests.el ends here
-- 
2.42.0

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

From: Davide Masserut <dm <at> mssdvd.com>
To: Davide Masserut <dm <at> mssdvd.com>
Cc: 65604 <at> debbugs.gnu.org, Jim Porter <jporterbugs <at> gmail.com>
Subject: Re: bug#65604: [PATCH] Display the exit code if the last command
 failed in Eshell
Date: Wed, 30 Aug 2023 17:26:27 +0200

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

Sorry, I forgot to make a variable buffer-local.

[0001-Display-the-exit-code-if-the-last-command-failed-in-.patch (text/x-patch, attachment)]

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

From: Eshel Yaron <me <at> eshelyaron.com>
To: Davide Masserut via "Bug reports for GNU Emacs, the Swiss army knife of
 text editors" <bug-gnu-emacs <at> gnu.org>
Cc: 65604 <at> debbugs.gnu.org, Jim Porter <jporterbugs <at> gmail.com>,
 Davide Masserut <dm <at> mssdvd.com>
Subject: Re: bug#65604: [PATCH] Display the exit code if the last command
 failed in Eshell
Date: Wed, 30 Aug 2023 17:34:51 +0200

Hello Davide,

This looks interesting, a couple of thoughts:

> From 8b9f3870e00cdf920e803d92138a9bb0f3c3b645 Mon Sep 17 00:00:00 2001
> From: Davide Masserut <dm <at> mssdvd.com>
> Date: Wed, 30 Aug 2023 16:38:07 +0200
> Subject: [PATCH] Display the exit code if the last command failed in Eshell
>

[...]

> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -370,6 +370,9 @@ to load the edited aliases.
>  Running 'rgrep' in Eshell now uses the Emacs grep facility instead of
>  calling external rgrep.
>  
> +---
> +*** If the last command failed, its exit code is now displayed in the modeline.
> +

I suggest a small rephrase to avoid the passive voice:
"Eshell now displays the exit code of the last command in the mode line
when it's non-zero."

Also, shouldn't this also be mentioned in the manual?

> +(defun eshell-update-last-command-status (exit-code)
> +  "Set `eshell-last-command-status' to EXIT-CODE and update `mode-line-process'."
> +  (setq mode-line-process
> +        (when (> exit-code 0)
> +            (list
> +             (let ((out-string (format ":[%s]" exit-code))
> +                   (msg (format "Last command exited with code %s" exit-code)))
> +               (propertize out-string
> +                           'help-echo msg
> +                           'face 'compilation-mode-line-fail))))
> +        eshell-last-command-status exit-code))
> +

You should be able to use an `:eval` mode line construct here instead of
resetting `mode-line-process` after each command this way.  This would
allow other code to extend modify `mode-line-process` as well, without
having the modification undone after each command.


Best,

Eshel (no relation to Eshell)

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: 65604 <at> debbugs.gnu.org, jporterbugs <at> gmail.com, dm <at> mssdvd.com
Subject: Re: bug#65604: [PATCH] Display the exit code if the last command
 failed in Eshell
Date: Wed, 30 Aug 2023 19:45:29 +0300

> Cc: jporterbugs <at> gmail.com, dm <at> mssdvd.com
> Date: Wed, 30 Aug 2023 17:34:51 +0200
> From:  Eshel Yaron via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
> 
> I suggest a small rephrase to avoid the passive voice:
> "Eshell now displays the exit code of the last command in the mode line
> when it's non-zero."

Always try to avoid possible confusion due to wrong attribution (does
"it's" refer to the mode line or to the last command or to the exit
code?).  So:

  If a command exits abnormally, Eshel now displays its exit code on
  the mode line.

> Also, shouldn't this also be mentioned in the manual?

Of course, it should!

> > +(defun eshell-update-last-command-status (exit-code)
> > +  "Set `eshell-last-command-status' to EXIT-CODE and update `mode-line-process'."
> > +  (setq mode-line-process
> > +        (when (> exit-code 0)
> > +            (list
> > +             (let ((out-string (format ":[%s]" exit-code))
> > +                   (msg (format "Last command exited with code %s" exit-code)))
> > +               (propertize out-string
> > +                           'help-echo msg
> > +                           'face 'compilation-mode-line-fail))))
> > +        eshell-last-command-status exit-code))
> > +
> 
> You should be able to use an `:eval` mode line construct here instead of
> resetting `mode-line-process` after each command this way.  This would
> allow other code to extend modify `mode-line-process` as well, without
> having the modification undone after each command.

Why do you meed :eval at all?  AFAIR, having a symbol in the mode line
automatically uses its current value when the mode line is redrawn.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: me <at> eshelyaron.com
Cc: 65604 <at> debbugs.gnu.org, jporterbugs <at> gmail.com, dm <at> mssdvd.com
Subject: Re: bug#65604: [PATCH] Display the exit code if the last command
 failed in Eshell
Date: Wed, 30 Aug 2023 19:58:27 +0300

> Cc: 65604 <at> debbugs.gnu.org, jporterbugs <at> gmail.com, dm <at> mssdvd.com
> Date: Wed, 30 Aug 2023 19:45:29 +0300
> From: Eli Zaretskii <eliz <at> gnu.org>
> 
> Always try to avoid possible confusion due to wrong attribution (does
> "it's" refer to the mode line or to the last command or to the exit
> code?).  So:
> 
>   If a command exits abnormally, Eshel now displays its exit code on
>   the mode line.

Even better:

  If a command exits abnormally, Eshel now displays the command's exit
  code on the mode line.

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

From: Davide Masserut <dm <at> mssdvd.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 65604 <at> debbugs.gnu.org, jporterbugs <at> gmail.com,
 Eshel Yaron <me <at> eshelyaron.com>
Subject: Re: bug#65604: [PATCH] Display the exit code if the last command
 failed in Eshell
Date: Wed, 30 Aug 2023 21:02:00 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

> Why do you meed :eval at all?  AFAIR, having a symbol in the 
> mode line
> automatically uses its current value when the mode line is 
> redrawn.

Wouldn't this strip the symbol of its text properties?

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Davide Masserut <dm <at> mssdvd.com>
Cc: 65604 <at> debbugs.gnu.org, jporterbugs <at> gmail.com, me <at> eshelyaron.com
Subject: Re: bug#65604: [PATCH] Display the exit code if the last command
 failed in Eshell
Date: Wed, 30 Aug 2023 22:25:34 +0300

> From: Davide Masserut <dm <at> mssdvd.com>
> Cc: Eshel Yaron <me <at> eshelyaron.com>, 65604 <at> debbugs.gnu.org,
>  jporterbugs <at> gmail.com
> Date: Wed, 30 Aug 2023 21:02:00 +0200
> 
> Eli Zaretskii <eliz <at> gnu.org> writes:
> 
> > Why do you meed :eval at all?  AFAIR, having a symbol in the 
> > mode line
> > automatically uses its current value when the mode line is 
> > redrawn.
> 
> Wouldn't this strip the symbol of its text properties?

Sorry, I don't understand: what symbol and why do we care about its
text properties?

What I meant is that reference to a symbol in mode-line-format
automatically uses the value of that symbol, unless I'm confused or
misremembering.

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

From: Davide Masserut <dm <at> mssdvd.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 65604 <at> debbugs.gnu.org, jporterbugs <at> gmail.com, me <at> eshelyaron.com
Subject: Re: bug#65604: [PATCH] Display the exit code if the last command
 failed in Eshell
Date: Wed, 30 Aug 2023 21:59:35 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

>> > Why do you meed :eval at all?  AFAIR, having a symbol in the
>> > mode line
>> > automatically uses its current value when the mode line is
>> > redrawn.
>>
>> Wouldn't this strip the symbol of its text properties?
>
> Sorry, I don't understand: what symbol and why do we care about 
> its
> text properties?
>
> What I meant is that reference to a symbol in mode-line-format
> automatically uses the value of that symbol, unless I'm confused 
> or
> misremembering.

"(elisp) Mode Line Data" says:

    Unless SYMBOL is marked as risky (i.e., it has a non-‘nil’
    ‘risky-local-variable’ property), all text properties 
    specified in
    SYMBOL’s value are ignored.  This includes the text 
    properties of
    strings in SYMBOL’s value, as well as all ‘:eval’ and 
    ‘:propertize’
    forms in it.  (The reason for this is security: non-risky 
    variables
    could be set automatically from file variables without 
    prompting
    the user.)


Given this code:

 (defun eshell-mode-line-exit-code ()
   (when (> eshell-last-command-status 0)
     (propertize
      (format ":[%s]" eshell-last-command-status)
      'help-echo (format "Last command exited with code %s"
                         eshell-last-command-status)
      'face 'compilation-mode-line-fail)))

 (setq-local mode-line-process 'eshell-mode-line-exit-code)

Doesn't it mean that unless we mark it "risky-local-variable", 
Emacs will remove the "compilation-mode-line-fail" face?

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

From: Davide Masserut <dm <at> mssdvd.com>
To: Eli Zaretskii <eliz <at> gnu.org>, me <at> eshelyaron.com, 65604 <at> debbugs.gnu.org,
 jporterbugs <at> gmail.com
Subject: Re: bug#65604: [PATCH] Display the exit code if the last command
 failed in Eshell
Date: Wed, 30 Aug 2023 22:20:56 +0200

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

I updated the NEWS file, the manual and used the ":eval" construct 
to update "mode-line-process".

[0001-Display-the-exit-code-if-the-last-command-failed-in-.patch (text/x-patch, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Davide Masserut <dm <at> mssdvd.com>
Cc: 65604 <at> debbugs.gnu.org, jporterbugs <at> gmail.com, me <at> eshelyaron.com
Subject: Re: bug#65604: [PATCH] Display the exit code if the last command
 failed in Eshell
Date: Thu, 31 Aug 2023 07:52:16 +0300

> From: Davide Masserut <dm <at> mssdvd.com>
> Cc: me <at> eshelyaron.com, 65604 <at> debbugs.gnu.org, jporterbugs <at> gmail.com
> Date: Wed, 30 Aug 2023 21:59:35 +0200
> 
>      Unless SYMBOL is marked as risky (i.e., it has a non-‘nil’
>      ‘risky-local-variable’ property), all text properties 
>      specified in
>      SYMBOL’s value are ignored.  This includes the text 
>      properties of
>      strings in SYMBOL’s value, as well as all ‘:eval’ and 
>      ‘:propertize’
>      forms in it.  (The reason for this is security: non-risky 
>      variables
>      could be set automatically from file variables without 
>      prompting
>      the user.)
> 
> 
> Given this code:
> 
>   (defun eshell-mode-line-exit-code ()
>     (when (> eshell-last-command-status 0)
>       (propertize
>        (format ":[%s]" eshell-last-command-status)
>        'help-echo (format "Last command exited with code %s"
>                           eshell-last-command-status)
>        'face 'compilation-mode-line-fail)))
> 
>   (setq-local mode-line-process 'eshell-mode-line-exit-code)
> 
> Doesn't it mean that unless we mark it "risky-local-variable", 
> Emacs will remove the "compilation-mode-line-fail" face?

Ah, that.  Yes, the properties will be ignored.  But do we really need
fancy faces on that display?

(FWIW, I personally don't like the idea of showing this on the mode
line: the mode line is already quite cramped, and OTOH Bash does show
abnormal exit codes as part of its prompt.  But feel free to disregard
my opinions, as I'm not a heavy user of Eshell.)

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

From: Davide Masserut <dm <at> mssdvd.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 65604 <at> debbugs.gnu.org, jporterbugs <at> gmail.com, me <at> eshelyaron.com
Subject: Re: bug#65604: [PATCH] Display the exit code if the last command
 failed in Eshell
Date: Thu, 31 Aug 2023 11:31:53 +0200

Eli Zaretskii <eliz <at> gnu.org> writes:

> Ah, that.  Yes, the properties will be ignored.  But do we 
> really need
> fancy faces on that display?

They make the error stand out.

> (FWIW, I personally don't like the idea of showing this on the 
> mode
> line: the mode line is already quite cramped

I agree with you, this is one of the main reasons I had in mind 
when I made the first patch.

> and OTOH Bash does show abnormal exit codes as part of its 
> prompt.  But feel free to disregard
> my opinions, as I'm not a heavy user of Eshell.)

fish and some zsh distributions also show the error in the prompt.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Davide Masserut <dm <at> mssdvd.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: 65604 <at> debbugs.gnu.org, me <at> eshelyaron.com
Subject: Re: bug#65604: [PATCH] Display the exit code if the last command
 failed in Eshell
Date: Fri, 1 Sep 2023 22:17:30 -0700

On 8/31/2023 2:31 AM, Davide Masserut via Bug reports for GNU Emacs, the 
Swiss army knife of text editors wrote:
>> (FWIW, I personally don't like the idea of showing this on the mode
>> line: the mode line is already quite cramped
> 
> I agree with you, this is one of the main reasons I had in mind when I 
> made the first patch.

Hmm, well if everyone else disagrees, I suppose I don't see any *major* 
issues with including the exit status in the prompt, though I'm a little 
worried it would annoy people who like the current way. If it were 
easier to customize the prompt, I don't think I'd be as worried. Fixing 
that for real is probably beyond the scope of this bug, but I do have a 
WIP patch for it.

If we do use the mode-line to display this though, I was initially 
thinking we could use the existing variable 
'eshell-command-running-string', which we could set to something like 
"!!" in 'eshell-command-finished'. That's not as useful as the actual 
number though, and honestly I'm not sure the current 
'eshell-status-in-mode-line' code is a good idea anyway. It 
(buffer-locally) replaces the mode-line construct immediately after 
'mode-line-front-space', which means you lose some of the 
potentially-useful information there.

Maybe it would make sense to move *all* of the current Eshell mode-line 
stuff to the 'mode-line-process' construct. That seems like it would be 
less brittle. That is, in 'mode-line-process', we could show whether a 
command is running or the exit status if nothing's running (possibly 
including successful exit). What does everyone think about that?

>> and OTOH Bash does show abnormal exit codes as part of its prompt.  
>> But feel free to disregard
>> my opinions, as I'm not a heavy user of Eshell.)
> 
> fish and some zsh distributions also show the error in the prompt.

Do they? I tried to see what the default was for Bash and after some 
searching, it seemed that it doesn't show the exit status by default. 
But that could be wrong.

In any case, sorry for the back and forth. This is another corner of 
Eshell I wasn't really aware of until this bug, so I'm not 100% sure 
what implementation follows the spirit of Eshell the most...

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

From: Davide Masserut <dm <at> mssdvd.com>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 65604 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, me <at> eshelyaron.com
Subject: Re: bug#65604: [PATCH] Display the exit code if the last command
 failed in Eshell
Date: Sat, 02 Sep 2023 10:47:15 +0200

Jim Porter <jporterbugs <at> gmail.com> writes:

> Hmm, well if everyone else disagrees, I suppose I don't see any
> *major* issues with including the exit status in the prompt, though
> I'm a little worried it would annoy people who like the current
> way. If it were easier to customize the prompt, I don't think I'd be
> as worried. Fixing that for real is probably beyond the scope of 
> this
> bug, but I do have a WIP patch for it.

What do you find difficult to customize?

I can only think of two things:

1) The default function is quite simple, but displaying the bytecode 
in customization buffer can be confusing.

Hide Eshell Prompt Function: Function:
#[0 "\300\301 !\302 \303U\203\0\304\202\0\305P\207"
   [abbreviate-file-name eshell/pwd file-user-uid 0 " # " " $ "] 3]
   State : STANDARD.
  A function that returns the Eshell prompt string. Hide
  Make sure to update ‘eshell-prompt-regexp’ so that it will match 
  your
  prompt.

2) It may require to update the regexp.

Can we use rx to make it more understandable?

> If we do use the mode-line to display this though, I was initially
> thinking we could use the existing variable
> 'eshell-command-running-string', which we could set to something 
> like
> "!!" in 'eshell-command-finished'. That's not as useful as the 
> actual
> number though, and honestly I'm not sure the current
> 'eshell-status-in-mode-line' code is a good idea anyway. It
> (buffer-locally) replaces the mode-line construct immediately after
> 'mode-line-front-space', which means you lose some of the
> potentially-useful information there.
>
> Maybe it would make sense to move *all* of the current Eshell
> mode-line stuff to the 'mode-line-process' construct. That seems 
> like
> it would be less brittle. That is, in 'mode-line-process', we could
> show whether a command is running or the exit status if nothing's
> running (possibly including successful exit). What does everyone 
> think
> about that?

In this case I would add a small delay before signaling that something 
is running.

However, I'm not sure it is so useful to signaling it.
When I suspect that the process is hanging, I usually use tools like 
top or proced to check if something is stuck, but otherwise I already 
know something is going on.

I believe some terminals update the title bar when something is 
running and send a notification when process ends and the window is 
not focused.
But for such long operations compilation-mode works best for me (BTW, 
thank you for the new compile command).

>>> and OTOH Bash does show abnormal exit codes as part of its prompt.
>>> But feel free to disregard
>>> my opinions, as I'm not a heavy user of Eshell.)
>> fish and some zsh distributions also show the error in the prompt.
>
> Do they? I tried to see what the default was for Bash and after some
> searching, it seemed that it doesn't show the exit status by
> default. But that could be wrong.

Distros often change the default prompt.

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

From: Jim Porter <jporterbugs <at> gmail.com>
To: Davide Masserut <dm <at> mssdvd.com>
Cc: 65604 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, me <at> eshelyaron.com
Subject: Re: bug#65604: [PATCH] Display the exit code if the last command
 failed in Eshell
Date: Sat, 2 Sep 2023 11:40:53 -0700

On 9/2/2023 1:47 AM, Davide Masserut via Bug reports for GNU Emacs, the 
Swiss army knife of text editors wrote:
> Jim Porter <jporterbugs <at> gmail.com> writes:
> 
>> Hmm, well if everyone else disagrees, I suppose I don't see any
>> *major* issues with including the exit status in the prompt, though
>> I'm a little worried it would annoy people who like the current
>> way. If it were easier to customize the prompt, I don't think I'd be
>> as worried. Fixing that for real is probably beyond the scope of this
>> bug, but I do have a WIP patch for it.
> 
> What do you find difficult to customize?
> 
> I can only think of two things:
> 
> 1) The default function is quite simple, but displaying the bytecode in 
> customization buffer can be confusing.

Yeah, mainly the Customize interface. It's a bit confusing, and when I 
think back to when I didn't understand Elisp, I would have found any 
customization of the Eshell prompt to be pretty intimidating.

I have a WIP patch for this though, which uses 'mode-line-format' to 
make the Eshell prompt work more like the mode-line. It seems to work 
pretty well, but I need to finish it up. (And especially to make sure it 
doesn't do anything weird with multi-line prompts.)

> 2) It may require to update the regexp.

The prompt regexp is (thankfully) almost irrelevant in Emacs 30 now. It 
only matters for paragraph-movement commands, which we could probably 
just remap to the actual Eshell-specific commands to navigate forward 
and backward through the prompts. I should probably just make a patch 
for this and finally get rid of that regexp entirely.

> In this case I would add a small delay before signaling that something 
> is running.

The delay isn't present in the current Eshell mode-line implementation, 
and I don't think anyone's raised an issue about that...

> However, I'm not sure it is so useful to signaling it.
> When I suspect that the process is hanging, I usually use tools like top 
> or proced to check if something is stuck, but otherwise I already know 
> something is going on.

I think there's some use, especially for longer-running Lisp functions, 
but that area is a bit of a mess anyway since it's tough to run Lisp 
code asynchronously (or at least, in a way that meshes with Eshell).

> I believe some terminals update the title bar when something is running 
> and send a notification when process ends and the window is not focused.
> But for such long operations compilation-mode works best for me (BTW, 
> thank you for the new compile command).

Yeah, this is roughly what I'm thinking for Eshell. The mode-line seems 
to me like a decent place for that info.

>> Do they? I tried to see what the default was for Bash and after some
>> searching, it seemed that it doesn't show the exit status by
>> default. But that could be wrong.
> 
> Distros often change the default prompt.

Yeah, I tried to take that into account when I looked it up. The actual 
default from GNU Bash is *very* basic, but what you get as a fresh user 
on most distros is a little fancier. I didn't see that it reported the 
exit status though.

... in any case, maybe the simplest way forward here is to put the 
(non-zero) exit status in the prompt like your original patch, and then 
separately, I can try to improve the customizability of the prompt, as 
well as thinking about what to do with the mode-line.

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

From: Davide Masserut <dm <at> mssdvd.com>
To: Jim Porter <jporterbugs <at> gmail.com>
Cc: 65604 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, me <at> eshelyaron.com
Subject: Re: bug#65604: [PATCH] Display the exit code if the last command
 failed in Eshell
Date: Sat, 02 Sep 2023 20:54:38 +0200

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

Jim Porter <jporterbugs <at> gmail.com> writes:

>> 2) It may require to update the regexp.
>
> The prompt regexp is (thankfully) almost irrelevant in Emacs 30
> now. It only matters for paragraph-movement commands, which we 
> could
> probably just remap to the actual Eshell-specific commands to 
> navigate
> forward and backward through the prompts. I should probably just 
> make
> a patch for this and finally get rid of that regexp entirely.

I didn't know that, thanks.

>> In this case I would add a small delay before signaling that
>> something is running.
>
> The delay isn't present in the current Eshell mode-line
> implementation, and I don't think anyone's raised an issue about
> that...

Sorry, I meant that if we decided to show a relative long message 
like "Running" in the mode-line, then we should show it only when 
it lasts for more than, let's say, 0.5s.  This would prevent the 
mode-line from moving to right for just a fraction of a second 
when running fast commands like cd or ls.

> ... in any case, maybe the simplest way forward here is to put 
> the
> (non-zero) exit status in the prompt like your original patch, 
> and
> then separately, I can try to improve the customizability of the
> prompt, as well as thinking about what to do with the mode-line.

I have updated the patch.

[0001-Display-the-exit-code-if-the-last-command-failed-in-.patch (text/x-patch, attachment)]

Message #61 received at 65604-done <at> debbugs.gnu.org (full text, mbox):

From: Jim Porter <jporterbugs <at> gmail.com>
To: Davide Masserut <dm <at> mssdvd.com>
Cc: 65604-done <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>, me <at> eshelyaron.com
Subject: Re: bug#65604: [PATCH] Display the exit code if the last command
 failed in Eshell
Date: Sat, 2 Sep 2023 15:46:11 -0700

Version: 30.1

On 9/2/2023 11:54 AM, Davide Masserut via Bug reports for GNU Emacs, the 
Swiss army knife of text editors wrote:
> Jim Porter <jporterbugs <at> gmail.com> writes:
> 
>> The prompt regexp is (thankfully) almost irrelevant in Emacs 30
>> now. It only matters for paragraph-movement commands, which we could
>> probably just remap to the actual Eshell-specific commands to navigate
>> forward and backward through the prompts. I should probably just make
>> a patch for this and finally get rid of that regexp entirely.
> 
> I didn't know that, thanks.

I've now made it fully-obsolete, so it doesn't do anything anymore (now, 
Eshell implements its own paragraph navigation based on fields).

> Sorry, I meant that if we decided to show a relative long message like 
> "Running" in the mode-line, then we should show it only when it lasts 
> for more than, let's say, 0.5s.  This would prevent the mode-line from 
> moving to right for just a fraction of a second when running fast 
> commands like cd or ls.

Hmm, that's true. One benefit of the current mode-line implementation 
for Eshell is that the indicator always takes up the same width. I'll 
keep thinking about this.

>> ... in any case, maybe the simplest way forward here is to put the
>> (non-zero) exit status in the prompt like your original patch, and
>> then separately, I can try to improve the customizability of the
>> prompt, as well as thinking about what to do with the mode-line.
> 
> I have updated the patch.

Thanks. I rebased it on top of my changes and pushed it to master as 
3d08d0dd80a. Closing this bug now.

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

From: Sean Whitton <spwhitton <at> spwhitton.name>
To: 65604 <at> debbugs.gnu.org
Cc: jporterbugs <at> gmail.com, dm <at> mssdvd.com
Subject: Re: bug#65604: [PATCH] Display the exit code if the last command
 failed in Eshell
Date: Sun, 10 Sep 2023 15:44:22 +0100

Hello,

Thank you both for this.  I've had this in my init.el for ages, and it
nice to have it there by default.

-- 
Sean Whitton

Previous Next

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