bug#54603: 29.0.50; [PATCH] Eshell's external pipe module interferes with other argument parsing hooks

[Sean Whitton <spwhitton@spwhitton.name>]

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

Hello,

On Thu 31 Mar 2022 at 04:31PM -07, Jim Porter wrote:

> On 3/31/2022 3:48 PM, Sean Whitton wrote:
>> 
>> I think the error should be caught inside the `or', though?  The idea
>> would be that if eshell-incomplete is thrown within one of the
>> disjuncts, that disjunct should return nil.
>
> Hmm, that's an interesting thought. Maybe this code could be more 
> particular about what parse function it calls. Since each of the 
> function calls here:
>
>                           (while (or (eshell-parse-lisp-argument)
>                                      (eshell-parse-backslash)
>                                      (eshell-parse-double-quote)
>                                      (eshell-parse-literal-quote)))
>
> correspond to a particular token here (earlier in the source):
>
>                                 (re-search-forward
>                                  "\\(?:(\\|#?'\\|\"\\|\\\\\\)" bound t)))
>
> perhaps it would be better to match the function call to the 
> corresponding token.

Thank you for this suggestion, but I think that findbeg1 is more
readable, and actually perhaps more efficient, if we maintain the 
(while (or ...)) structure.

So I would like to install the attached patch to resolve this bug.

-- 
Sean Whitton

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-em-extpipe-Catch-eshell-incomplete-thrown-while-pars.patch --]
[-- Type: text/x-patch, Size: 3500 bytes --]

From a66fd4c30779a64e56621d21e589bb35856a57ea Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Fri, 1 Apr 2022 14:05:07 -0700
Subject: [PATCH] em-extpipe: Catch eshell-incomplete thrown while parsing

* lisp/eshell/em-extpipe.el (em-extpipe--or-with-catch): New macro.
(eshell-parse-external-pipeline): Use new macro to treat
`eshell-incomplete' as a failure of the parse function to move us
forward (Bug#54603).  Thanks to Jim Porter <jporterbugs@gmail.com> for
the report and for help isolating the problem.

* test/lisp/eshell/eshell-tests.el
(eshell-test/lisp-command-with-quote): New test for Bug#54603, thanks
to Jim Porter <jporterbugs@gmail.com>.
---
 lisp/eshell/em-extpipe.el        | 22 ++++++++++++++++++----
 test/lisp/eshell/eshell-tests.el |  4 ++++
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/lisp/eshell/em-extpipe.el b/lisp/eshell/em-extpipe.el
index eb5b3bfe1d..3db1dea595 100644
--- a/lisp/eshell/em-extpipe.el
+++ b/lisp/eshell/em-extpipe.el
@@ -49,6 +49,19 @@ eshell-extpipe-initialize
   (add-hook 'eshell-pre-rewrite-command-hook
             #'eshell-rewrite-external-pipeline -20 t))
 
+(defmacro em-extpipe--or-with-catch (&rest disjuncts)
+  "Evaluate DISJUNCTS like `or' but catch `eshell-incomplete'.
+
+If `eshell-incomplete' is thrown during the evaluation of a
+disjunct, that disjunct yields nil."
+  (let ((result (gensym)))
+    `(let (,result)
+       (or ,@(cl-loop for disjunct in disjuncts collect
+                      `(if (catch 'eshell-incomplete
+                             (ignore (setq ,result ,disjunct)))
+                           nil
+                         ,result))))))
+
 (defun eshell-parse-external-pipeline ()
   "Parse a pipeline intended for execution by the external shell.
 
@@ -105,10 +118,11 @@ eshell-parse-external-pipeline
                        (if (re-search-forward pat next t)
                            (throw 'found (match-beginning 1))
                          (goto-char next)
-                         (while (or (eshell-parse-lisp-argument)
-                                    (eshell-parse-backslash)
-                                    (eshell-parse-double-quote)
-                                    (eshell-parse-literal-quote)))
+                         (while (em-extpipe--or-with-catch
+                                 (eshell-parse-lisp-argument)
+                                 (eshell-parse-backslash)
+                                 (eshell-parse-double-quote)
+                                 (eshell-parse-literal-quote)))
                          ;; Guard against an infinite loop if none of
                          ;; the parsers moved us forward.
                          (unless (or (> (point) next) (eobp))
diff --git a/test/lisp/eshell/eshell-tests.el b/test/lisp/eshell/eshell-tests.el
index e31db07c61..1e303f70e5 100644
--- a/test/lisp/eshell/eshell-tests.el
+++ b/test/lisp/eshell/eshell-tests.el
@@ -44,6 +44,10 @@ eshell-test/lisp-command
   "Test `eshell-command-result' with an elisp command."
   (should (equal (eshell-test-command-result "(+ 1 2)") 3)))
 
+(ert-deftest eshell-test/lisp-command-with-quote ()
+  "Test `eshell-command-result' with an elisp command containing a quote."
+  (should (equal (eshell-test-command-result "(eq 'foo nil)") nil)))
+
 (ert-deftest eshell-test/for-loop ()
   "Test `eshell-command-result' with a for loop.."
   (let ((process-environment (cons "foo" process-environment)))
-- 
2.30.2