From: Jim Porter <jporterbugs@gmail.com>
To: Sean Whitton <spwhitton@spwhitton.name>, 54603@debbugs.gnu.org
Subject: bug#54603: 29.0.50; [PATCH] Eshell's external pipe module interferes with other argument parsing hooks
Date: Thu, 31 Mar 2022 16:31:53 -0700 [thread overview]
Message-ID: <8191b68d-975d-3f84-bb0b-501bdf2edf47@gmail.com> (raw)
In-Reply-To: <87y20pbuwu.fsf@athena.silentflame.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
next prev parent reply other threads:[~2022-03-31 23:31 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-03-28 2:21 bug#54603: 29.0.50; [PATCH] Eshell's external pipe module interferes with other argument parsing hooks Jim Porter
2022-03-31 11:46 ` Lars Ingebrigtsen
2022-03-31 16:26 ` Jim Porter
2022-03-31 16:50 ` Eli Zaretskii
2022-03-31 17:35 ` Michael Albinus
2022-03-31 22:55 ` Sean Whitton
2022-03-31 18:26 ` Sean Whitton
2022-03-31 20:58 ` Jim Porter
2022-03-31 21:55 ` Sean Whitton
2022-03-31 22:19 ` Jim Porter
2022-03-31 22:48 ` Sean Whitton
2022-03-31 23:31 ` Jim Porter [this message]
2022-04-01 21:16 ` Sean Whitton
2022-04-02 1:31 ` Jim Porter
2022-04-02 14:09 ` Lars Ingebrigtsen
2022-04-02 15:52 ` Sean Whitton
2022-03-31 21:56 ` Sean Whitton
2022-03-31 23:11 ` Jim Porter
2022-04-16 21:04 ` Sean Whitton
2022-05-23 4:34 ` Jim Porter
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
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8191b68d-975d-3f84-bb0b-501bdf2edf47@gmail.com \
--to=jporterbugs@gmail.com \
--cc=54603@debbugs.gnu.org \
--cc=spwhitton@spwhitton.name \
/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 external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.