unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Jim Porter <jporterbugs@gmail.com>
To: 55620@debbugs.gnu.org
Subject: bug#55620: [PATCH] 29.0.50; Eshell doesn't reset `eshell-in-pipeline-p' in subcommands
Date: Tue, 24 May 2022 20:37:01 -0700	[thread overview]
Message-ID: <a6c2d1e1-70c5-43e6-4f14-2b0c5cfb98b5@gmail.com> (raw)
In-Reply-To: <b7cd14b8-3473-5d9f-2848-67a9b29e392d@gmail.com>

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

(Originally mis-sent with the wrong email address.)

Attached is a patch with tests to fix this. Note that I reset 
`eshell-in-pipeline-p' in two places:

1. `eshell-subcommand-bindings': This handles any case where a 
subcommand is invoked. This can happen with the following syntaxes: 
{CMD}, ${CMD}, and $<CMD>.

2. `eshell-command-to-value': This handles cases where a command is run 
synchronously. This can happen with some of the syntaxes mentioned 
above: {CMD} and ${CMD}. However, it can also happen for Lisp forms like 
(LISP) and $(LISP). (Most Lisp forms aren't likely to consult 
`eshell-in-pipeline-p', but if you were calling an Eshell built-in 
command using Lisp syntax, you might run into this issue.)

[-- Attachment #2: 0001-Reset-eshell-in-pipeline-p-when-interpolating-comman.patch --]
[-- Type: text/plain, Size: 3340 bytes --]

From 3a18199fcc8919220892c25c64970c7343c9e63f Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Tue, 24 May 2022 18:56:50 -0700
Subject: [PATCH] Reset 'eshell-in-pipeline-p' when interpolating commands

* lisp/eshell/esh-cmd.el (eshell-subcommand-bindings)
(eshell-command-to-value): Set 'eshell-in-pipeline-p' to nil.

* test/lisp/eshell/eshell-tests.el
(eshell-test/subcommand-reset-in-pipeline)
(eshell-test/lisp-reset-in-pipeline): New tests (bug#55620).
---
 lisp/eshell/esh-cmd.el           |  4 +++-
 test/lisp/eshell/eshell-tests.el | 29 +++++++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index 73c250632c..775e4c1057 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -256,6 +256,7 @@ eshell-deferrable-commands
 
 (defcustom eshell-subcommand-bindings
   '((eshell-in-subcommand-p t)
+    (eshell-in-pipeline-p nil)
     (default-directory default-directory)
     (process-environment (eshell-copy-environment)))
   "A list of `let' bindings for subcommand environments."
@@ -907,7 +908,8 @@ eshell-do-command-to-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")))
+  `(let ((value (make-symbol "eshell-temp"))
+         (eshell-in-pipeline-p nil))
      (eshell-do-command-to-value ,object)))
 
 ;;;_* Iterative evaluation
diff --git a/test/lisp/eshell/eshell-tests.el b/test/lisp/eshell/eshell-tests.el
index c0affed80a..ab5d73d479 100644
--- a/test/lisp/eshell/eshell-tests.el
+++ b/test/lisp/eshell/eshell-tests.el
@@ -130,6 +130,35 @@ eshell-test/pipe-subcommand-with-pipe
    (eshell-command-result-p "echo ${*echo hi | *cat} | *cat"
                             "hi")))
 
+(ert-deftest eshell-test/subcommand-reset-in-pipeline ()
+  "Check that subcommands reset `eshell-in-pipeline-p'."
+  (skip-unless (executable-find "cat"))
+  (dolist (template '("echo {%s} | *cat"
+                      "echo ${%s} | *cat"
+                      "*cat $<%s> | *cat"))
+    (should (equal (eshell-test-command-result
+                    (format template "echo $eshell-in-pipeline-p"))
+                   nil))
+    (should (equal (eshell-test-command-result
+                    (format template "echo | echo $eshell-in-pipeline-p"))
+                   "last"))
+    (should (equal (eshell-test-command-result
+                    (format template "echo $eshell-in-pipeline-p | echo"))
+                   "first"))
+    (should (equal (eshell-test-command-result
+                    (format template
+                            "echo | echo $eshell-in-pipeline-p | echo"))
+                   "t"))))
+
+(ert-deftest eshell-test/lisp-reset-in-pipeline ()
+  "Check that interpolated Lisp forms reset `eshell-in-pipeline-p'."
+  (skip-unless (executable-find "cat"))
+  (dolist (template '("echo (%s) | *cat"
+                      "echo $(%s) | *cat"))
+    (should (equal (eshell-test-command-result
+                    (format template "format \"%s\" eshell-in-pipeline-p"))
+                   "nil"))))
+
 (ert-deftest eshell-test/redirect-buffer ()
   "Check that piping to a buffer works"
   (with-temp-buffer
-- 
2.25.1


  reply	other threads:[~2022-05-25  3:37 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-25  2:20 bug#55620: 29.0.50; Eshell doesn't reset `eshell-in-pipeline-p' in subcommands Jim Porter
2022-05-25  3:37 ` Jim Porter [this message]
2022-05-25 12:13   ` Lars Ingebrigtsen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a6c2d1e1-70c5-43e6-4f14-2b0c5cfb98b5@gmail.com \
    --to=jporterbugs@gmail.com \
    --cc=55620@debbugs.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).