bug#65604: [PATCH] Display the exit code if the last command failed in Eshell

bug#65604: [PATCH] Display the exit code if the last command failed in Eshell

[Davide Masserut via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 891 bytes --]

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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Display-the-exit-code-if-the-last-command-failed-in-.patch --]
[-- Type: text/patch, Size: 5492 bytes --]

From a0e2f7bda33767e8ce104dd32c76e4d3d6c1a2f8 Mon Sep 17 00:00:00 2001
From: Davide Masserut <dm@mssdvd.com>
Date: Tue, 29 Aug 2023 22:33:48 +0200
Subject: [PATCH] Display the exit code if the last command failed in Eshell

* etc/NEWS: Announce change.
* lisp/eshell/em-prompt.el (eshell-prompt-function): Insert the exit
code if last command failed.
* lisp/eshell/esh-io.el (eshell-last-command-status): Make it
buffer-local.
* test/lisp/eshell/em-prompt-tests.el (em-prompt-test/after-failure)
(em-prompt-test/next-previous-prompt-with)
(em-prompt-test/forward-backward-matching-input-with): New tests.
---
 etc/NEWS                            |  3 +++
 lisp/eshell/em-prompt.el            |  2 ++
 lisp/eshell/esh-io.el               |  2 +-
 test/lisp/eshell/em-prompt-tests.el | 37 +++++++++++++++++++++++++----
 4 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 9a98db8c83a..9622e57f476 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.
 
+---
+*** The Eshell prompt now shows the exit code if the last command failed.
+
 ** Pcomplete
 
 ---
diff --git a/lisp/eshell/em-prompt.el b/lisp/eshell/em-prompt.el
index 42f8f273b52..692b579d02d 100644
--- a/lisp/eshell/em-prompt.el
+++ b/lisp/eshell/em-prompt.el
@@ -50,6 +50,8 @@ eshell-prompt-load-hook
 (defcustom eshell-prompt-function
   (lambda ()
     (concat (abbreviate-file-name (eshell/pwd))
+            (unless (eshell-exit-success-p)
+              (format " [%d]" eshell-last-command-status))
             (if (= (file-user-uid) 0) " # " " $ ")))
   "A function that returns the Eshell prompt string.
 Make sure to update `eshell-prompt-regexp' so that it will match your
diff --git a/lisp/eshell/esh-io.el b/lisp/eshell/esh-io.el
index c07f871dd37..cd0cee6e21d 100644
--- a/lisp/eshell/esh-io.el
+++ b/lisp/eshell/esh-io.el
@@ -170,7 +170,7 @@ eshell-redirection-operators-alist
 
 (defvar eshell-current-handles nil)
 
-(defvar eshell-last-command-status 0
+(defvar-local eshell-last-command-status 0
   "The exit code from the last command.  0 if successful.")
 
 (defvar eshell-last-command-result nil
diff --git a/test/lisp/eshell/em-prompt-tests.el b/test/lisp/eshell/em-prompt-tests.el
index 93bf9d84ab3..c90f417cefd 100644
--- a/test/lisp/eshell/em-prompt-tests.el
+++ b/test/lisp/eshell/em-prompt-tests.el
@@ -34,6 +34,25 @@
 
 ;;; Tests:
 
+(ert-deftest em-prompt-test/after-failure ()
+  "Check that current prompt shows the exit code of the last failed command."
+  (with-temp-eshell
+   (let ((debug-on-error nil))
+     (eshell-insert-command "(zerop \"foo\")"))
+   (let ((current-prompt (field-string (1- (point)))))
+     (should (equal-including-properties
+              current-prompt
+              (propertize
+               (concat (directory-file-name default-directory)
+                       (unless (eshell-exit-success-p)
+                         (format " [%d]" eshell-last-command-status))
+                       (if (= (file-user-uid) 0) " # " " $ "))
+               'read-only t
+               'field 'prompt
+               'font-lock-face 'eshell-prompt
+               'front-sticky '(read-only field font-lock-face)
+               'rear-nonsticky '(read-only field font-lock-face)))))))
+
 (ert-deftest em-prompt-test/field-properties ()
   "Check that field properties are properly set on Eshell output/prompts."
   (with-temp-eshell
@@ -88,6 +107,8 @@ em-prompt-test--with-multiline
 (defun em-prompt-test/next-previous-prompt-with ()
   "Helper for checking forward/backward navigation of old prompts."
   (with-temp-eshell
+   (let ((debug-on-error nil))
+     (eshell-insert-command "(zerop \"foo\")")) ; A failed command.
    (eshell-insert-command "echo one")
    (eshell-insert-command "echo two")
    (eshell-insert-command "echo three")
@@ -99,8 +120,11 @@ em-prompt-test/next-previous-prompt-with
    (end-of-line)
    (eshell-previous-prompt 2)
    (should (equal (eshell-get-old-input) "echo one"))
-   ;; Go forward three prompts.
-   (eshell-next-prompt 3)
+   ;; Go back one prompt.
+   (eshell-previous-prompt 1)
+   (should (equal (eshell-get-old-input) "(zerop \"foo\")"))
+   ;; Go forward four prompts.
+   (eshell-next-prompt 4)
    (should (equal (eshell-get-old-input) "echo fou"))))
 
 (ert-deftest em-prompt-test/next-previous-prompt ()
@@ -115,6 +139,8 @@ em-prompt-test/next-previous-prompt-multiline
 (defun em-prompt-test/forward-backward-matching-input-with ()
   "Helper for checking forward/backward navigation via regexps."
   (with-temp-eshell
+   (let ((debug-on-error nil))
+     (eshell-insert-command "(zerop \"foo\")")) ; A failed command.
    (eshell-insert-command "echo one")
    (eshell-insert-command "printnl something else")
    (eshell-insert-command "echo two")
@@ -127,8 +153,11 @@ em-prompt-test/forward-backward-matching-input-with
    (end-of-line)
    (eshell-backward-matching-input "echo" 2)
    (should (equal (eshell-get-old-input) "echo one"))
-   ;; Go forward three prompts.
-   (eshell-forward-matching-input "echo" 3)
+   ;; Go back one prompt.
+   (eshell-previous-prompt 1)
+   (should (equal (eshell-get-old-input) "(zerop \"foo\")"))
+   ;; Go forward four prompts.
+   (eshell-forward-matching-input "echo" 4)
    (should (equal (eshell-get-old-input) "echo fou"))))
 
 (ert-deftest em-prompt-test/forward-backward-matching-input ()
-- 
2.42.0

bug#65604: [PATCH] Display the exit code if the last command failed in Eshell

[Jim Porter]

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

bug#65604: [PATCH] Display the exit code if the last command failed in Eshell

[Davide Masserut via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 676 bytes --]

Jim Porter <jporterbugs@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.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Display-the-exit-code-if-the-last-command-failed-in-.patch --]
[-- Type: text/x-patch, Size: 5045 bytes --]

From 8b9f3870e00cdf920e803d92138a9bb0f3c3b645 Mon Sep 17 00:00:00 2001
From: Davide Masserut <dm@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

bug#65604: [PATCH] Display the exit code if the last command failed in Eshell

[Davide Masserut via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 50 bytes --]

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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Display-the-exit-code-if-the-last-command-failed-in-.patch --]
[-- Type: text/x-patch, Size: 5328 bytes --]

From d418c7a9a88cc2c5aff2e0ff7ad9681c0c0016bb Mon Sep 17 00:00:00 2001
From: Davide Masserut <dm@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            | 17 +++++++++++++++--
 test/lisp/eshell/esh-io-tests.el | 15 +++++++++++++++
 4 files changed, 37 insertions(+), 6 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..96ce08051b4 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))
@@ -170,7 +171,7 @@ eshell-redirection-operators-alist
 
 (defvar eshell-current-handles nil)
 
-(defvar eshell-last-command-status 0
+(defvar-local eshell-last-command-status 0
   "The exit code from the last command.  0 if successful.")
 
 (defvar eshell-last-command-result nil
@@ -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

bug#65604: [PATCH] Display the exit code if the last command failed in Eshell

[Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hello Davide,

This looks interesting, a couple of thoughts:

> From 8b9f3870e00cdf920e803d92138a9bb0f3c3b645 Mon Sep 17 00:00:00 2001
> From: Davide Masserut <dm@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)

bug#65604: [PATCH] Display the exit code if the last command failed in Eshell

[Eli Zaretskii]

> Cc: jporterbugs@gmail.com, dm@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@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.

bug#65604: [PATCH] Display the exit code if the last command failed in Eshell

[Eli Zaretskii]

> Cc: 65604@debbugs.gnu.org, jporterbugs@gmail.com, dm@mssdvd.com
> Date: Wed, 30 Aug 2023 19:45:29 +0300
> From: Eli Zaretskii <eliz@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.

bug#65604: [PATCH] Display the exit code if the last command failed in Eshell

[Davide Masserut via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@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?

bug#65604: [PATCH] Display the exit code if the last command failed in Eshell

[Eli Zaretskii]

> From: Davide Masserut <dm@mssdvd.com>
> Cc: Eshel Yaron <me@eshelyaron.com>, 65604@debbugs.gnu.org,
>  jporterbugs@gmail.com
> Date: Wed, 30 Aug 2023 21:02:00 +0200
> 
> Eli Zaretskii <eliz@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.

bug#65604: [PATCH] Display the exit code if the last command failed in Eshell

[Davide Masserut via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@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?

bug#65604: [PATCH] Display the exit code if the last command failed in Eshell

[Davide Masserut via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 100 bytes --]

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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Display-the-exit-code-if-the-last-command-failed-in-.patch --]
[-- Type: text/x-patch, Size: 4486 bytes --]

From 33e97ff350e1184f97c2867bd689a77291a2c7fd Mon Sep 17 00:00:00 2001
From: Davide Masserut <dm@mssdvd.com>
Date: Wed, 30 Aug 2023 21:34:08 +0200
Subject: [PATCH] Display the exit code if the last command failed in Eshell

* doc/misc/eshell.texi (Invocation): Document change.
* etc/NEWS: Announce change.
* lisp/eshell/esh-io.el (eshell-last-command-status): Make it buffer-local.
* lisp/eshell/esh-mode.el (compile): Require it for a face.
(eshell-mode): Update mode-line-process based on the exit code of the
last command.
* test/lisp/eshell/esh-io-tests.el (eshell)
(esh-io-test/modeline-after-failure): Add test.
---
 doc/misc/eshell.texi             |  3 +++
 etc/NEWS                         |  3 +++
 lisp/eshell/esh-io.el            |  2 +-
 lisp/eshell/esh-mode.el          | 12 ++++++++++++
 test/lisp/eshell/esh-io-tests.el | 16 ++++++++++++++++
 5 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index f8f60bae13a..b544e0cda41 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -234,6 +234,9 @@ Invocation
 can be controlled the same way as any other background process in
 Emacs.
 
+If a command exits abnormally, Eshell displays the command's exit code
+on the mode line.
+
 @subsection Command form
 Command form looks much the same as in other shells.  A command
 consists of arguments separated by spaces; the first argument is the
diff --git a/etc/NEWS b/etc/NEWS
index 9a98db8c83a..fe81cfc2477 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 a command exits abnormally, Eshell now displays the command's exit code on the mode line.
+
 ** Pcomplete
 
 ---
diff --git a/lisp/eshell/esh-io.el b/lisp/eshell/esh-io.el
index c07f871dd37..cd0cee6e21d 100644
--- a/lisp/eshell/esh-io.el
+++ b/lisp/eshell/esh-io.el
@@ -170,7 +170,7 @@ eshell-redirection-operators-alist
 
 (defvar eshell-current-handles nil)
 
-(defvar eshell-last-command-status 0
+(defvar-local eshell-last-command-status 0
   "The exit code from the last command.  0 if successful.")
 
 (defvar eshell-last-command-result nil
diff --git a/lisp/eshell/esh-mode.el b/lisp/eshell/esh-mode.el
index 0c381dbb86a..71ff901c764 100644
--- a/lisp/eshell/esh-mode.el
+++ b/lisp/eshell/esh-mode.el
@@ -69,6 +69,8 @@
 (require 'esh-util)
 (require 'esh-var)
 
+(require 'compile)
+
 (defgroup eshell-mode nil
   "This module contains code for handling input from the user."
   :tag "User interface"
@@ -369,6 +371,16 @@ eshell-mode
   ;; strong R2L character.
   (setq bidi-paragraph-direction 'left-to-right)
 
+  (setq-local
+   mode-line-process
+   '(:eval
+     (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))))
+
   ;; load extension modules into memory.  This will cause any global
   ;; variables they define to be visible, since some of the core
   ;; modules sometimes take advantage of their functionality if used.
diff --git a/test/lisp/eshell/esh-io-tests.el b/test/lisp/eshell/esh-io-tests.el
index ce80f3a8f08..349c4332d82 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,19 @@ 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
+            '(:eval
+              (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)))))))
+
 ;;; esh-io-tests.el ends here
-- 
2.42.0

bug#65604: [PATCH] Display the exit code if the last command failed in Eshell

[Eli Zaretskii]

> From: Davide Masserut <dm@mssdvd.com>
> Cc: me@eshelyaron.com, 65604@debbugs.gnu.org, jporterbugs@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.)

bug#65604: [PATCH] Display the exit code if the last command failed in Eshell

[Davide Masserut via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@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.

bug#65604: [PATCH] Display the exit code if the last command failed in Eshell

[Jim Porter]

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

bug#65604: [PATCH] Display the exit code if the last command failed in Eshell

[Davide Masserut via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Jim Porter <jporterbugs@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.

bug#65604: [PATCH] Display the exit code if the last command failed in Eshell

[Jim Porter]

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

bug#65604: [PATCH] Display the exit code if the last command failed in Eshell

[Davide Masserut via Bug reports for GNU Emacs, the Swiss army knife of text editors]

[-- Attachment #1: Type: text/plain, Size: 1314 bytes --]

Jim Porter <jporterbugs@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.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Display-the-exit-code-if-the-last-command-failed-in-.patch --]
[-- Type: text/x-patch, Size: 6120 bytes --]

From 83f716950a60ee64c49654033430f8e09d373f24 Mon Sep 17 00:00:00 2001
From: Davide Masserut <dm@mssdvd.com>
Date: Tue, 29 Aug 2023 22:33:48 +0200
Subject: [PATCH] Display the exit code if the last command failed in Eshell

* doc/misc/eshell.texi (Invocation): Document change.
* etc/NEWS: Announce change.
* lisp/eshell/em-prompt.el (eshell-prompt-function): Insert the exit
code if last command failed.
* lisp/eshell/esh-io.el (eshell-last-command-status): Make it
buffer-local.
* test/lisp/eshell/em-prompt-tests.el (em-prompt-test/after-failure)
(em-prompt-test/next-previous-prompt-with)
(em-prompt-test/forward-backward-matching-input-with): New tests.
---
 doc/misc/eshell.texi                |  3 +++
 etc/NEWS                            |  3 +++
 lisp/eshell/em-prompt.el            |  2 ++
 lisp/eshell/esh-io.el               |  2 +-
 test/lisp/eshell/em-prompt-tests.el | 37 +++++++++++++++++++++++++----
 5 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 7a563bc794c..677bd0f3e3e 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -234,6 +234,9 @@ Invocation
 can be controlled the same way as any other background process in
 Emacs.
 
+If a command exits abnormally, Eshell displays the command's exit code
+in the prompt.
+
 @subsection Command form
 Command form looks much the same as in other shells.  A command
 consists of arguments separated by spaces; the first argument is the
diff --git a/etc/NEWS b/etc/NEWS
index 5c11b6b9ac7..7a9010a2a9a 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 a command exits abnormally, Eshell now displays the command's exit code in the prompt.
+
 ** Pcomplete
 
 ---
diff --git a/lisp/eshell/em-prompt.el b/lisp/eshell/em-prompt.el
index 42f8f273b52..692b579d02d 100644
--- a/lisp/eshell/em-prompt.el
+++ b/lisp/eshell/em-prompt.el
@@ -50,6 +50,8 @@ eshell-prompt-load-hook
 (defcustom eshell-prompt-function
   (lambda ()
     (concat (abbreviate-file-name (eshell/pwd))
+            (unless (eshell-exit-success-p)
+              (format " [%d]" eshell-last-command-status))
             (if (= (file-user-uid) 0) " # " " $ ")))
   "A function that returns the Eshell prompt string.
 Make sure to update `eshell-prompt-regexp' so that it will match your
diff --git a/lisp/eshell/esh-io.el b/lisp/eshell/esh-io.el
index c07f871dd37..cd0cee6e21d 100644
--- a/lisp/eshell/esh-io.el
+++ b/lisp/eshell/esh-io.el
@@ -170,7 +170,7 @@ eshell-redirection-operators-alist
 
 (defvar eshell-current-handles nil)
 
-(defvar eshell-last-command-status 0
+(defvar-local eshell-last-command-status 0
   "The exit code from the last command.  0 if successful.")
 
 (defvar eshell-last-command-result nil
diff --git a/test/lisp/eshell/em-prompt-tests.el b/test/lisp/eshell/em-prompt-tests.el
index 93bf9d84ab3..c90f417cefd 100644
--- a/test/lisp/eshell/em-prompt-tests.el
+++ b/test/lisp/eshell/em-prompt-tests.el
@@ -34,6 +34,25 @@
 
 ;;; Tests:
 
+(ert-deftest em-prompt-test/after-failure ()
+  "Check that current prompt shows the exit code of the last failed command."
+  (with-temp-eshell
+   (let ((debug-on-error nil))
+     (eshell-insert-command "(zerop \"foo\")"))
+   (let ((current-prompt (field-string (1- (point)))))
+     (should (equal-including-properties
+              current-prompt
+              (propertize
+               (concat (directory-file-name default-directory)
+                       (unless (eshell-exit-success-p)
+                         (format " [%d]" eshell-last-command-status))
+                       (if (= (file-user-uid) 0) " # " " $ "))
+               'read-only t
+               'field 'prompt
+               'font-lock-face 'eshell-prompt
+               'front-sticky '(read-only field font-lock-face)
+               'rear-nonsticky '(read-only field font-lock-face)))))))
+
 (ert-deftest em-prompt-test/field-properties ()
   "Check that field properties are properly set on Eshell output/prompts."
   (with-temp-eshell
@@ -88,6 +107,8 @@ em-prompt-test--with-multiline
 (defun em-prompt-test/next-previous-prompt-with ()
   "Helper for checking forward/backward navigation of old prompts."
   (with-temp-eshell
+   (let ((debug-on-error nil))
+     (eshell-insert-command "(zerop \"foo\")")) ; A failed command.
    (eshell-insert-command "echo one")
    (eshell-insert-command "echo two")
    (eshell-insert-command "echo three")
@@ -99,8 +120,11 @@ em-prompt-test/next-previous-prompt-with
    (end-of-line)
    (eshell-previous-prompt 2)
    (should (equal (eshell-get-old-input) "echo one"))
-   ;; Go forward three prompts.
-   (eshell-next-prompt 3)
+   ;; Go back one prompt.
+   (eshell-previous-prompt 1)
+   (should (equal (eshell-get-old-input) "(zerop \"foo\")"))
+   ;; Go forward four prompts.
+   (eshell-next-prompt 4)
    (should (equal (eshell-get-old-input) "echo fou"))))
 
 (ert-deftest em-prompt-test/next-previous-prompt ()
@@ -115,6 +139,8 @@ em-prompt-test/next-previous-prompt-multiline
 (defun em-prompt-test/forward-backward-matching-input-with ()
   "Helper for checking forward/backward navigation via regexps."
   (with-temp-eshell
+   (let ((debug-on-error nil))
+     (eshell-insert-command "(zerop \"foo\")")) ; A failed command.
    (eshell-insert-command "echo one")
    (eshell-insert-command "printnl something else")
    (eshell-insert-command "echo two")
@@ -127,8 +153,11 @@ em-prompt-test/forward-backward-matching-input-with
    (end-of-line)
    (eshell-backward-matching-input "echo" 2)
    (should (equal (eshell-get-old-input) "echo one"))
-   ;; Go forward three prompts.
-   (eshell-forward-matching-input "echo" 3)
+   ;; Go back one prompt.
+   (eshell-previous-prompt 1)
+   (should (equal (eshell-get-old-input) "(zerop \"foo\")"))
+   ;; Go forward four prompts.
+   (eshell-forward-matching-input "echo" 4)
    (should (equal (eshell-get-old-input) "echo fou"))))
 
 (ert-deftest em-prompt-test/forward-backward-matching-input ()
-- 
2.42.0

bug#65604: [PATCH] Display the exit code if the last command failed in Eshell

[Jim Porter]

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

bug#65604: [PATCH] Display the exit code if the last command failed in Eshell

[Sean Whitton]

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