From: Jim Porter Date: Sun, 19 Mar 2023 02:18:28 +0000 (-0700) Subject: Avoid shadowing variables in some Eshell command forms X-Git-Url: http://git.eshelyaron.com/gitweb/?a=commitdiff_plain;h=97e35b149874a105a9975853a7fcd6f0034ddeab;p=emacs.git Avoid shadowing variables in some Eshell command forms * lisp/eshell/esh-cmd.el (eshell-rewrite-for-command): Make 'for-items' an uninterned symbol. (eshell-as-subcommand): Correct docstring. (eshell-do-command-to-value): Mark obsolete. (eshell-command-to-value): Move binding of 'value' outside of the macro's result, and remove call to 'eshell-do-command-to-value'. * test/lisp/eshell/esh-cmd-tests.el (esh-cmd-test/subcommand-shadow-value) (esh-cmd-test/for-loop-for-items-shadow): New tests. (esh-cmd-test/for-name-loop, esh-cmd-test/for-name-shadow-loop): Rename to... (esh-cmd-test/for-loop-name, esh-cmd-test/for-loop-name-shadow): ... these. --- diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el index d5237ee1f04..f0c6a146dfd 100644 --- a/lisp/eshell/esh-cmd.el +++ b/lisp/eshell/esh-cmd.el @@ -542,9 +542,10 @@ of its argument (i.e., use of a Lisp special form), it must be implemented via rewriting, rather than as a function." (if (and (equal (car terms) "for") (equal (nth 2 terms) "in")) - (let ((body (car (last terms)))) + (let ((for-items (make-symbol "for-items")) + (body (car (last terms)))) (setcdr (last terms 2) nil) - `(let ((for-items + `(let ((,for-items (append ,@(mapcar (lambda (elem) @@ -552,13 +553,13 @@ implemented via rewriting, rather than as a function." elem `(list ,elem))) (nthcdr 3 terms))))) - (while for-items - (let ((,(intern (cadr terms)) (car for-items)) + (while ,for-items + (let ((,(intern (cadr terms)) (car ,for-items)) (eshell--local-vars (cons ',(intern (cadr terms)) eshell--local-vars))) (eshell-protect ,(eshell-invokify-arg body t))) - (setq for-items (cdr for-items))) + (setq ,for-items (cdr ,for-items))) (eshell-close-handles))))) (defun eshell-structure-basic-command (func names keyword test body @@ -901,28 +902,33 @@ This is used on systems where async subprocesses are not supported." (symbol-value tailproc)))))) (defmacro eshell-as-subcommand (command) - "Execute COMMAND using a temp buffer. -This is used so that certain Lisp commands, such as `cd', when -executed in a subshell, do not disturb the environment of the main -Eshell buffer." + "Execute COMMAND as a subcommand. +A subcommand creates a local environment so that any changes to +the environment don't propagate outside of the subcommand's +scope. This lets you use commands like `cd' within a subcommand +without changing the current directory of the main Eshell +buffer." `(let ,eshell-subcommand-bindings ,command)) (defmacro eshell-do-command-to-value (object) "Run a subcommand prepared by `eshell-command-to-value'. This avoids the need to use `let*'." + (declare (obsolete nil "30.1")) `(let ((eshell-current-handles (eshell-create-handles value 'overwrite))) (progn ,object (symbol-value value)))) -(defmacro eshell-command-to-value (object) - "Run OBJECT synchronously, returning its result as a string. -Returns a string comprising the output from the command." - `(let ((value (make-symbol "eshell-temp")) - (eshell-in-pipeline-p nil)) - (eshell-do-command-to-value ,object))) +(defmacro eshell-command-to-value (command) + "Run an Eshell COMMAND synchronously, returning its output." + (let ((value (make-symbol "eshell-temp"))) + `(let ((eshell-in-pipeline-p nil) + (eshell-current-handles + (eshell-create-handles ',value 'overwrite))) + ,command + ,value))) ;;;_* Iterative evaluation ;; diff --git a/test/lisp/eshell/esh-cmd-tests.el b/test/lisp/eshell/esh-cmd-tests.el index 94763954622..a7208eb3a0b 100644 --- a/test/lisp/eshell/esh-cmd-tests.el +++ b/test/lisp/eshell/esh-cmd-tests.el @@ -73,6 +73,13 @@ Test that trailing arguments outside the subcommand are ignored. e.g. \"{(+ 1 2)} 3\" => 3" (eshell-command-result-equal "{(+ 1 2)} 3" 3)) +(ert-deftest esh-cmd-test/subcommand-shadow-value () + "Test that the variable `value' isn't shadowed inside subcommands." + (with-temp-eshell + (with-no-warnings (setq-local value "hello")) + (eshell-match-command-output "echo ${echo $value}" + "hello\n"))) + (ert-deftest esh-cmd-test/let-rebinds-after-defer () "Test that let-bound values are properly updated after `eshell-defer'. When inside a `let' block in an Eshell command form, we need to @@ -151,13 +158,13 @@ bug#59469." (eshell-match-command-output "for i in 1 2 (list 3 4) { echo $i }" "1\n2\n3\n4\n"))) -(ert-deftest esh-cmd-test/for-name-loop () ; bug#15231 +(ert-deftest esh-cmd-test/for-loop-name () ; bug#15231 "Test invocation of a for loop using `name'." (let ((process-environment (cons "name" process-environment))) (eshell-command-result-equal "for name in 3 { echo $name }" 3))) -(ert-deftest esh-cmd-test/for-name-shadow-loop () ; bug#15372 +(ert-deftest esh-cmd-test/for-loop-name-shadow () ; bug#15372 "Test invocation of a for loop using an env-var." (let ((process-environment (cons "name=env-value" process-environment))) (with-temp-eshell @@ -165,6 +172,13 @@ bug#59469." "echo $name; for name in 3 { echo $name }; echo $name" "env-value\n3\nenv-value\n")))) +(ert-deftest esh-cmd-test/for-loop-for-items-shadow () + "Test that the variable `for-items' isn't shadowed inside for loops." + (with-temp-eshell + (with-no-warnings (setq-local for-items "hello")) + (eshell-match-command-output "for i in 1 { echo $for-items }" + "hello\n"))) + (ert-deftest esh-cmd-test/for-loop-pipe () "Test invocation of a for loop piped to another command." (skip-unless (executable-find "rev"))