Subject: Re: bug#26161: 25.1; `eshell-exit-success-p' determines that Lisp commands are successful if they return non-nil

From: "George D. Plymale" <[email-masked]>
Date: Tue, 11 Apr 2017 15:48:05 -0400

--=-=-=
Content-Type: text/x-patch
Content-Disposition: attachment; filename=bug_26161_fix.diff
Content-Description: Fix for bug#26161

diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index 583ba6ac42..86e7b83c28 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -575,14 +575,9 @@ eshell-rewrite-if-command
(defvar eshell-last-command-result)     ;Defined in esh-io.el.

(defun eshell-exit-success-p ()
-  "Return non-nil if the last command was \"successful\".
-For a bit of Lisp code, this means a return value of non-nil.
-For an external command, it means an exit code of 0."
-  (if (save-match-data
-	(string-match "#<\\(Lisp object\\|function .*\\)>"
-		      eshell-last-command-name))
-      eshell-last-command-result
-    (= eshell-last-command-status 0)))
+  "Return non-nil if the last command was successful.
+This means an exit code of 0."
+  (= eshell-last-command-status 0))

(defvar eshell--cmd)

@@ -1257,6 +1252,7 @@ eshell-exec-lisp
         (and result (funcall printer result))
         result)
     (error
+     (setq eshell-last-command-status 1)
      (let ((msg (error-message-string err)))
        (if (and (not form-p)
                 (string-match "^Wrong number of arguments" msg)

--=-=-=
Content-Type: text/plain

Okay, so I created a patch to fix this issue. Go easy on me since this
is my first patch to Emacs ;)

The changes are pretty simple:

- `eshell-exit-success-p' has been changed to only check if the last
  exit code was zero, rather than first checking whether the last command
  returned nil.
- `eshell-exec-lisp' has been changed so that it will set
  `eshell-last-command-status' to 1 if it catches an error.
- These changes together make it so that the `&&' operator in Eshell
  behaves more expectedly to someone who has used a bash-like shell and
  so that other things involving the success of Lisp commands in Eshell
  are more reliable.
  
Feel free to point out anything else that should be done here or any
errors on my part. I have tested these changes out and everything seems
okay. I can run the aforementioned commands that were problematic. Examples:

~ $ cd .. && pwd
/Users

/Users $ cd - && pwd
/Users/my_username

~ $ .. && pwd
/Users

/Users $ cd - && pwd
/Users/my_username

~ $ cat ~/.emacs && pwd
;; Emacs init file stuff
(foo bar)/Users/my_username

~ $ cat nowhere && pwd
cat: nowhere: No such file or directory

~ $ cat nowhere ; pwd
cat: nowhere: No such file or directory
/Users/my_username

~ $ 

--=-=-=--