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

[Jim Porter <jporterbugs@gmail.com>]

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

On 3/31/2022 3:48 PM, Sean Whitton wrote:
> Thank you for testing.  In the meantime, I've thought it all through
> some more, and I think that your original idea of catching the
> 'eshell-incomplete is the right thing, assuming all tests pass.
> 
> 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. That is, if we see a "#?", we call 
`eshell-parse-lisp-argument', and so on. See the attached patch, which 
works in my tests (and passes all the existing Eshell unit tests).

[-- Attachment #2: 0001-Make-Eshell-s-extpipe-more-lenient-when-looking-for-.patch --]
[-- Type: text/plain, Size: 3034 bytes --]

From d3ffc4b10cc37a8babc3048ef3507d4d74c5fc2f Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 27 Mar 2022 19:04:42 -0700
Subject: [PATCH] Make Eshell's extpipe more lenient when looking for its
 operators

This could cause some errors when executing Eshell commands with an
odd number of quotation marks, such as "(eq 'foo nil)".

* lisp/eshell/em-extpipe.el (eshell-parse-external-pipeline): Catch
'eshell-incomplete' errors and only call the parse function
corresponding to the token at point.

* test/lisp/eshell/eshell-tests.el
(eshell-test/lisp-command-with-quote): New test.
---
 lisp/eshell/em-extpipe.el        | 11 +++++++----
 test/lisp/eshell/eshell-tests.el |  4 ++++
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/lisp/eshell/em-extpipe.el b/lisp/eshell/em-extpipe.el
index eb5b3bfe1d..c587f318bf 100644
--- a/lisp/eshell/em-extpipe.el
+++ b/lisp/eshell/em-extpipe.el
@@ -100,15 +100,18 @@ eshell-parse-external-pipeline
                              (save-excursion
                                (re-search-forward
                                 "\\(?:#?'\\|\"\\|\\\\\\)" bound t)))
+                            (token (and found (match-string 0)))
                             (next (or (and found (match-beginning 0))
                                       bound)))
                        (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)))
+                         (catch 'eshell-incomplete
+                           (pcase token
+                             ("#'" (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.25.1