From f3408af0a3251a744cb0b55b5e153565bfd57ea3 Mon Sep 17 00:00:00 2001 From: Jim Porter Date: Tue, 9 Aug 2022 20:09:57 -0700 Subject: [PATCH] Make '$?' and '$$' variables more consistent in Eshell Previously, '$?' (last exit code) was only useful for external commands, and '$$' (last result) was only useful for Lisp commands. * lisp/eshell/esh-cmd.el (eshell-lisp-form-nil-is-failure): New option. (eshell-lisp-command): Set last exit code to 1 when the command signals an error, and 2 if it returns nil (for Lisp forms only). * lisp/eshell/esh-proc.el (eshell-sentinel): Set last result to t if the command succeeded. * test/lisp/eshell/esh-cmd-tests.el (esh-cmd-test/while-loop-lisp-form, esh-cmd-test/until-loop-lisp-form) (esh-cmd-test/if-else-statement-lisp-form) (esh-cmd-test/if-else-statement-lisp-form-2) (esh-cmd-test/unless-else-statement-lisp-form): New tests. * test/lisp/eshell/esh-var-tests.el (esh-var-test/last-status-var-lisp-command) (esh-var-test/last-status-var-lisp-form) (esh-var-test/last-status-var-lisp-form-2) (esh-var-test/last-status-var-ext-cmd) (esh-var-test/last-status-var-ext-cmd): New tests. (esh-var-test/last-result-var2): Rename from this... ( esh-var-test/last-result-var-twice): ... to this. * doc/misc/eshell.texi (Variables): Update documentation about '$?' and '$$'. (Control Flow): Mention that '(lisp forms)' can be used as conditionals. * etc/NEWS: Announce this change (bug#57129). --- doc/misc/eshell.texi | 14 ++++--- etc/NEWS | 7 ++++ lisp/eshell/esh-cmd.el | 68 +++++++++++++++++++------------ lisp/eshell/esh-proc.el | 68 +++++++++++++++---------------- test/lisp/eshell/esh-cmd-tests.el | 54 ++++++++++++++++++++++++ test/lisp/eshell/esh-var-tests.el | 56 ++++++++++++++++++++++++- 6 files changed, 199 insertions(+), 68 deletions(-) diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi index 141c30ae9b9..aae779575d5 100644 --- a/doc/misc/eshell.texi +++ b/doc/misc/eshell.texi @@ -890,14 +890,18 @@ command (excluding the command name itself). @vindex $$ @item $$ -This is the result of the last command. In case of an external -command, it is @code{t} or @code{nil}. +This is the result of the last command. For external commands, it is +@code{t} if the exit code was 0 or @code{nil} otherwise. +@vindex eshell-lisp-form-nil-is-failure @vindex $? @item $? This variable contains the exit code of the last command. If the last command was a Lisp function, it is 0 for successful completion or 1 -otherwise. +otherwise. If @code{eshell-lisp-form-nil-is-failure} is +non-@code{nil}, then a command with a Lisp form, like +@samp{(@var{command} @var{args}@dots{})}, that returns @code{nil} will +set this variable to 2. @vindex $COLUMNS @vindex $LINES @@ -1024,8 +1028,8 @@ Most of Eshell's control flow statements accept a @var{conditional}. This can take a few different forms. If @var{conditional} is a dollar expansion, the condition is satisfied if the result is a non-@code{nil} value. If @var{conditional} is a @samp{@{ -@var{subcommand} @}}, the condition is satisfied if the -@var{subcommand}'s exit status is 0. +@var{subcommand} @}} or @samp{(@var{lisp form})}, the condition is +satisfied if the command's exit status is 0. @table @code diff --git a/etc/NEWS b/etc/NEWS index be647f6bbbc..f876916bb6f 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -2162,6 +2162,13 @@ Additionally, globs ending with '**/' or '***/' no longer raise an error, and now expand to all directories recursively (following symlinks in the latter case). ++++ +*** Lisp forms in Eshell now treat a 'nil' result as a failed exit status. +When executing a command that looks like '(lisp form)', Eshell will +set the exit status (available in the '$?' variable) to 2. This +allows commands like that to be used as conditionals. To change this +behavior, customize the new 'eshell-lisp-form-nil-is-failure' option. + ** Shell --- diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el index 454a90e91d3..62c95056fd2 100644 --- a/lisp/eshell/esh-cmd.el +++ b/lisp/eshell/esh-cmd.el @@ -133,6 +133,10 @@ There are several different kinds of commands, however." Such arguments will be passed to `read', and then evaluated." :type 'regexp) +(defcustom eshell-lisp-form-nil-is-failure t + "If non-nil, Lisp forms like (COMMAND ARGS) treat a nil result as failure." + :type 'boolean) + (defcustom eshell-pre-command-hook nil "A hook run before each interactive command is invoked." :type 'hook) @@ -1412,43 +1416,53 @@ via `eshell-errorn'." (defun eshell-lisp-command (object &optional args) "Insert Lisp OBJECT, using ARGS if a function." (catch 'eshell-external ; deferred to an external command + (setq eshell-last-command-status 0 + eshell-last-arguments args) (let* ((eshell-ensure-newline-p (eshell-interactive-output-p)) + (command-form-p (functionp object)) (result - (if (functionp object) - (progn - (setq eshell-last-arguments args - eshell-last-command-name + (if command-form-p + (let ((numeric (not (get object + 'eshell-no-numeric-conversions))) + (fname-args (get object 'eshell-filename-arguments))) + (when (or numeric fname-args) + (while args + (let ((arg (car args))) + (cond + ((and numeric (stringp arg) (> (length arg) 0) + (text-property-any 0 (length arg) + 'number t arg)) + ;; If any of the arguments are flagged as + ;; numbers waiting for conversion, convert + ;; them now. + (setcar args (string-to-number arg))) + ((and fname-args (stringp arg) + (string-equal arg "~")) + ;; If any of the arguments match "~", + ;; prepend "./" to treat it as a regular + ;; file name. + (setcar args (concat "./" arg))))) + (setq args (cdr args)))) + (setq eshell-last-command-name (concat "#")) - (let ((numeric (not (get object - 'eshell-no-numeric-conversions))) - (fname-args (get object 'eshell-filename-arguments))) - (when (or numeric fname-args) - (while args - (let ((arg (car args))) - (cond ((and numeric (stringp arg) (> (length arg) 0) - (text-property-any 0 (length arg) - 'number t arg)) - ;; If any of the arguments are - ;; flagged as numbers waiting for - ;; conversion, convert them now. - (setcar args (string-to-number arg))) - ((and fname-args (stringp arg) - (string-equal arg "~")) - ;; If any of the arguments match "~", - ;; prepend "./" to treat it as a - ;; regular file name. - (setcar args (concat "./" arg))))) - (setq args (cdr args))))) (eshell-apply object eshell-last-arguments)) - (setq eshell-last-arguments args - eshell-last-command-name "#") + (setq eshell-last-command-name "#") (eshell-eval object)))) (if (and eshell-ensure-newline-p (save-excursion (goto-char eshell-last-output-end) (not (bolp)))) (eshell-print "\n")) - (eshell-close-handles 0 (list 'quote result))))) + (eshell-close-handles + ;; If `eshell-lisp-form-nil-is-failure' is non-nil, Lisp forms + ;; that succeeded but have a nil result should have an exit + ;; status of 2. + (when (and eshell-lisp-form-nil-is-failure + (not command-form-p) + (= eshell-last-command-status 0) + (not result)) + 2) + (list 'quote result))))) (defalias 'eshell-lisp-command* #'eshell-lisp-command) diff --git a/lisp/eshell/esh-proc.el b/lisp/eshell/esh-proc.el index 99b43661f2c..c367b5cd643 100644 --- a/lisp/eshell/esh-proc.el +++ b/lisp/eshell/esh-proc.el @@ -346,7 +346,9 @@ Used only on systems which do not support async subprocesses.") (defvar eshell-last-output-end) ;Defined in esh-mode.el. (eshell-update-markers eshell-last-output-end) ;; Simulate the effect of eshell-sentinel. - (eshell-close-handles (if (numberp exit-status) exit-status -1)) + (eshell-close-handles + (if (numberp exit-status) exit-status -1) + (list 'quote (and (numberp exit-status) (= exit-status 0)))) (eshell-kill-process-function command exit-status) (or (bound-and-true-p eshell-in-pipeline-p) (setq eshell-last-sync-output-start nil)) @@ -398,40 +400,36 @@ PROC is the process that's exiting. STRING is the exit message." (when (buffer-live-p (process-buffer proc)) (with-current-buffer (process-buffer proc) (unwind-protect - (let ((entry (assq proc eshell-process-list))) -; (if (not entry) -; (error "Sentinel called for unowned process `%s'" -; (process-name proc)) - (when entry - (unwind-protect - (progn - (unless (string= string "run") - ;; Write the exit message if the status is - ;; abnormal and the process is already writing - ;; to the terminal. - (when (and (eq proc (eshell-tail-process)) - (not (string-match "^\\(finished\\|exited\\)" - string))) - (funcall (process-filter proc) proc string)) - (let ((handles (nth 1 entry)) - (str (prog1 (nth 3 entry) - (setf (nth 3 entry) nil))) - (status (process-exit-status proc))) - ;; If we're in the middle of handling output - ;; from this process then schedule the EOF for - ;; later. - (letrec ((finish-io - (lambda () - (if (nth 4 entry) - (run-at-time 0 nil finish-io) - (when str - (ignore-error 'eshell-pipe-broken - (eshell-output-object - str nil handles))) - (eshell-close-handles - status 'nil handles))))) - (funcall finish-io))))) - (eshell-remove-process-entry entry)))) + (when-let ((entry (assq proc eshell-process-list))) + (unwind-protect + (unless (string= string "run") + ;; Write the exit message if the status is + ;; abnormal and the process is already writing + ;; to the terminal. + (when (and (eq proc (eshell-tail-process)) + (not (string-match "^\\(finished\\|exited\\)" + string))) + (funcall (process-filter proc) proc string)) + (let ((handles (nth 1 entry)) + (str (prog1 (nth 3 entry) + (setf (nth 3 entry) nil))) + (status (process-exit-status proc))) + ;; If we're in the middle of handling output + ;; from this process then schedule the EOF for + ;; later. + (letrec ((finish-io + (lambda () + (if (nth 4 entry) + (run-at-time 0 nil finish-io) + (when str + (ignore-error 'eshell-pipe-broken + (eshell-output-object + str nil handles))) + (eshell-close-handles + status (list 'quote (= status 0)) + handles))))) + (funcall finish-io)))) + (eshell-remove-process-entry entry))) (eshell-kill-process-function proc string))))) (defun eshell-process-interact (func &optional all query) diff --git a/test/lisp/eshell/esh-cmd-tests.el b/test/lisp/eshell/esh-cmd-tests.el index b31159a1a8f..e86985ec717 100644 --- a/test/lisp/eshell/esh-cmd-tests.el +++ b/test/lisp/eshell/esh-cmd-tests.el @@ -139,6 +139,15 @@ e.g. \"{(+ 1 2)} 3\" => 3" "{ setq eshell-test-value (cdr eshell-test-value) }") "(1 2)\n(2)\n")))) +(ert-deftest esh-cmd-test/while-loop-lisp-form () + "Test invocation of a while loop using a Lisp form." + (with-temp-eshell + (let ((eshell-test-value 0)) + (eshell-command-result-p + (concat "while (/= eshell-test-value 3) " + "{ setq eshell-test-value (1+ eshell-test-value) }") + "1\n2\n3\n")))) + (ert-deftest esh-cmd-test/while-loop-ext-cmd () "Test invocation of a while loop using an external command." (skip-unless (executable-find "[")) @@ -158,6 +167,16 @@ e.g. \"{(+ 1 2)} 3\" => 3" "{ setq eshell-test-value t }") "t\n")))) +(ert-deftest esh-cmd-test/until-loop-lisp-form () + "Test invocation of an until loop using a Lisp form." + (skip-unless (executable-find "[")) + (with-temp-eshell + (let ((eshell-test-value 0)) + (eshell-command-result-p + (concat "until (= eshell-test-value 3) " + "{ setq eshell-test-value (1+ eshell-test-value) }") + "1\n2\n3\n")))) + (ert-deftest esh-cmd-test/until-loop-ext-cmd () "Test invocation of an until loop using an external command." (skip-unless (executable-find "[")) @@ -188,6 +207,30 @@ e.g. \"{(+ 1 2)} 3\" => 3" (eshell-command-result-p "if $eshell-test-value {echo yes} {echo no}" "no\n")))) +(ert-deftest esh-cmd-test/if-else-statement-lisp-form () + "Test invocation of an if/else statement using a Lisp form." + (with-temp-eshell + (eshell-command-result-p "if (zerop 0) {echo yes} {echo no}" + "yes\n") + (eshell-command-result-p "if (zerop 1) {echo yes} {echo no}" + "no\n") + (let ((debug-on-error nil)) + (eshell-command-result-p "if (zerop \"foo\") {echo yes} {echo no}" + "no\n")))) + +(ert-deftest esh-cmd-test/if-else-statement-lisp-form-2 () + "Test invocation of an if/else statement using a Lisp form. +This tests when `eshell-lisp-form-nil-is-failure' is nil." + (let ((eshell-lisp-form-nil-is-failure nil)) + (with-temp-eshell + (eshell-command-result-p "if (zerop 0) {echo yes} {echo no}" + "yes\n") + (eshell-command-result-p "if (zerop 1) {echo yes} {echo no}" + "yes\n") + (let ((debug-on-error nil)) + (eshell-command-result-p "if (zerop \"foo\") {echo yes} {echo no}" + "no\n"))))) + (ert-deftest esh-cmd-test/if-else-statement-ext-cmd () "Test invocation of an if/else statement using an external command." (skip-unless (executable-find "[")) @@ -217,6 +260,17 @@ e.g. \"{(+ 1 2)} 3\" => 3" (eshell-command-result-p "unless $eshell-test-value {echo no} {echo yes}" "no\n")))) +(ert-deftest esh-cmd-test/unless-else-statement-lisp-form () + "Test invocation of an unless/else statement using a Lisp form." + (with-temp-eshell + (eshell-command-result-p "unless (zerop 0) {echo no} {echo yes}" + "yes\n") + (eshell-command-result-p "unless (zerop 1) {echo no} {echo yes}" + "no\n") + (let ((debug-on-error nil)) + (eshell-command-result-p "unless (zerop \"foo\") {echo no} {echo yes}" + "no\n")))) + (ert-deftest esh-cmd-test/unless-else-statement-ext-cmd () "Test invocation of an unless/else statement using an external command." (skip-unless (executable-find "[")) diff --git a/test/lisp/eshell/esh-var-tests.el b/test/lisp/eshell/esh-var-tests.el index 54e701a6aab..66dabd424bd 100644 --- a/test/lisp/eshell/esh-var-tests.el +++ b/test/lisp/eshell/esh-var-tests.el @@ -500,18 +500,72 @@ inside double-quotes" (eshell-command-result-p "echo $INSIDE_EMACS[, 1]" "eshell"))) +(ert-deftest esh-var-test/last-status-var-lisp-command () + "Test using the \"last exit status\" ($?) variable with a Lisp command" + (with-temp-eshell + (eshell-command-result-p "zerop 0; echo $?" + "t\n0\n") + (eshell-command-result-p "zerop 1; echo $?" + "0\n") + (let ((debug-on-error nil)) + (eshell-command-result-p "zerop foo; echo $?" + "1\n")))) + +(ert-deftest esh-var-test/last-status-var-lisp-form () + "Test using the \"last exit status\" ($?) variable with a Lisp form" + (let ((eshell-lisp-form-nil-is-failure t)) + (with-temp-eshell + (eshell-command-result-p "(zerop 0); echo $?" + "t\n0\n") + (eshell-command-result-p "(zerop 1); echo $?" + "2\n") + (let ((debug-on-error nil)) + (eshell-command-result-p "(zerop \"foo\"); echo $?" + "1\n"))))) + +(ert-deftest esh-var-test/last-status-var-lisp-form-2 () + "Test using the \"last exit status\" ($?) variable with a Lisp form. +This tests when `eshell-lisp-form-nil-is-failure' is nil." + (let ((eshell-lisp-form-nil-is-failure nil)) + (with-temp-eshell + (eshell-command-result-p "(zerop 0); echo $?" + "0\n") + (eshell-command-result-p "(zerop 0); echo $?" + "0\n") + (let ((debug-on-error nil)) + (eshell-command-result-p "(zerop \"foo\"); echo $?" + "1\n"))))) + +(ert-deftest esh-var-test/last-status-var-ext-cmd () + "Test using the \"last exit status\" ($?) variable with an external command" + (skip-unless (executable-find "[")) + (with-temp-eshell + (eshell-command-result-p "[ foo = foo ]; echo $?" + "0\n") + (eshell-command-result-p "[ foo = bar ]; echo $?" + "1\n"))) + (ert-deftest esh-var-test/last-result-var () "Test using the \"last result\" ($$) variable" (with-temp-eshell (eshell-command-result-p "+ 1 2; + $$ 2" "3\n5\n"))) -(ert-deftest esh-var-test/last-result-var2 () +(ert-deftest esh-var-test/last-result-var-twice () "Test using the \"last result\" ($$) variable twice" (with-temp-eshell (eshell-command-result-p "+ 1 2; + $$ $$" "3\n6\n"))) +(ert-deftest esh-var-test/last-result-var-ext-cmd () + "Test using the \"last result\" ($$) variable with an external command" + (skip-unless (executable-find "[")) + (with-temp-eshell + (eshell-command-result-p "[ foo = foo ]; format \"%s\" $$" + "t\n") + (eshell-command-result-p "[ foo = bar ]; format \"%s\" $$" + "nil\n"))) + (ert-deftest esh-var-test/last-result-var-split-indices () "Test using the \"last result\" ($$) variable with split indices" (with-temp-eshell -- 2.39.5