bug#54682: 29.0.50; [PATCH] Improve Eshell's logic for finding delimiters in arguments

bug#54682: 29.0.50; [PATCH] Improve Eshell's logic for finding delimiters in arguments

[Jim Porter]

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

 From "emacs -Q --eval '(eshell)'":

   ;; Good:
   ~ $ echo "\\foo"
   \foo

   ;; Bad:
   ~ $ echo "\\"
   Expecting completion of delimiter " ...

This is an issue with the logic for `eshell-find-delimiter'. When seeing 
the first '\', it didn't recognize that the second '\' was being 
escaped. Then, it looked at the second '\' and saw that it preceded a 
'"', thus thinking that the '"' was escaped and the string was 
unterminated. The fix is that `eshell-find-delimiter' should treat '\' 
as an escapable character when using backslash escapes.

In addition, I fixed the docstring. Previously, it said that backslash 
escaping only occurred when BACKSLASH-P was non-nil *and* the opening 
and closing delimiters are the same. However, when the opening and 
closing delimiters are different, backslash escaping is always used.

[-- Attachment #2: 0001-Fix-handling-of-inside-double-quotes-in-Eshell.patch --]
[-- Type: text/plain, Size: 4685 bytes --]

From 566a1d7eeab6a1fa2babc54609f96e441cd45efc Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sun, 27 Mar 2022 12:09:58 -0700
Subject: [PATCH] Fix handling of '\\' inside double-quotes in Eshell

Previously, Eshell would get confused and think the following command
was unterminated due to the second double-quote looking like it was
escaped:

  echo "\\"

* lisp/eshell/esh-util.el (eshell-find-delimiter): Correct docstring
and treat '\' as an escapeable character when using backslash escapes.

* test/lisp/eshell/eshell-tests.el
(eshell-test/escape-special-quoted): Adapt test.
---
 lisp/eshell/esh-util.el          | 51 +++++++++++++++++---------------
 test/lisp/eshell/eshell-tests.el |  4 +--
 2 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/lisp/eshell/esh-util.el b/lisp/eshell/esh-util.el
index 788404fc43..8089d4d74b 100644
--- a/lisp/eshell/esh-util.el
+++ b/lisp/eshell/esh-util.el
@@ -151,49 +151,52 @@ eshell-condition-case
 (defun eshell-find-delimiter
   (open close &optional bound reverse-p backslash-p)
   "From point, find the CLOSE delimiter corresponding to OPEN.
-The matching is bounded by BOUND.
-If REVERSE-P is non-nil, process the region backwards.
-If BACKSLASH-P is non-nil, and OPEN and CLOSE are the same character,
-then quoting is done by a backslash, rather than a doubled delimiter."
+The matching is bounded by BOUND. If REVERSE-P is non-nil,
+process the region backwards.
+
+If BACKSLASH-P is non-nil, or OPEN and CLOSE are different
+characters, then a backslash can be used to escape a delimiter
+(or another backslash).  Otherwise, the delimiter is escaped by
+doubling it up."
   (save-excursion
     (let ((depth 1)
 	  (bound (or bound (point-max))))
-      (if (if reverse-p
-	      (eq (char-before) close)
-	    (eq (char-after) open))
-	  (forward-char (if reverse-p -1 1)))
+      (when (if reverse-p
+                (eq (char-before) close)
+              (eq (char-after) open))
+        (forward-char (if reverse-p -1 1)))
       (while (and (> depth 0)
-		  (funcall (if reverse-p '> '<) (point) bound))
-	(let ((c (if reverse-p (char-before) (char-after))) nc)
+                  (funcall (if reverse-p #'> #'<) (point) bound))
+        (let ((c (if reverse-p (char-before) (char-after))))
 	  (cond ((and (not reverse-p)
 		      (or (not (eq open close))
 			  backslash-p)
 		      (eq c ?\\)
-		      (setq nc (char-after (1+ (point))))
-		      (or (eq nc open) (eq nc close)))
+                      (memq (char-after (1+ (point)))
+                            (list open close ?\\)))
 		 (forward-char 1))
 		((and reverse-p
 		      (or (not (eq open close))
 			  backslash-p)
-		      (or (eq c open) (eq c close))
-		      (eq (char-before (1- (point)))
-			  ?\\))
+                      (eq (char-before (1- (point))) ?\\)
+                      (memq c (list open close ?\\)))
 		 (forward-char -1))
 		((eq open close)
-		 (if (eq c open)
-		     (if (and (not backslash-p)
-			      (eq (if reverse-p
-				      (char-before (1- (point)))
-				    (char-after (1+ (point)))) open))
-			 (forward-char (if reverse-p -1 1))
-		       (setq depth (1- depth)))))
+                 (when (eq c open)
+                   (if (and (not backslash-p)
+                            (eq (if reverse-p
+                                    (char-before (1- (point)))
+                                  (char-after (1+ (point))))
+                                open))
+                       (forward-char (if reverse-p -1 1))
+                     (setq depth (1- depth)))))
 		((= c open)
 		 (setq depth (+ depth (if reverse-p -1 1))))
 		((= c close)
 		 (setq depth (+ depth (if reverse-p 1 -1))))))
 	(forward-char (if reverse-p -1 1)))
-      (if (= depth 0)
-	  (if reverse-p (point) (1- (point)))))))
+      (when (= depth 0)
+        (if reverse-p (point) (1- (point)))))))
 
 (defun eshell-convert (string)
   "Convert STRING into a more native looking Lisp object."
diff --git a/test/lisp/eshell/eshell-tests.el b/test/lisp/eshell/eshell-tests.el
index 1e303f70e5..bcc2dc320b 100644
--- a/test/lisp/eshell/eshell-tests.el
+++ b/test/lisp/eshell/eshell-tests.el
@@ -148,9 +148,9 @@ eshell-test/escape-special-quoted
   "Test that the backslash is not preserved for escaped special
 chars"
   (with-temp-eshell
-   (eshell-command-result-p "echo \"h\\\\i\""
+   (eshell-command-result-p "echo \"\\\"hi\\\\\""
                             ;; Backslashes are doubled for regexp.
-                            "h\\\\i\n")))
+                            "\\\"hi\\\\\n")))
 
 (ert-deftest eshell-test/command-running-p ()
   "Modeline should show no command running"
-- 
2.25.1

bug#54682: 29.0.50; [PATCH] Improve Eshell's logic for finding delimiters in arguments

[Lars Ingebrigtsen]

Jim Porter <jporterbugs@gmail.com> writes:

> In addition, I fixed the docstring. Previously, it said that backslash
> escaping only occurred when BACKSLASH-P was non-nil *and* the opening
> and closing delimiters are the same. However, when the opening and
> closing delimiters are different, backslash escaping is always used.

Thanks; pushed to Emacs 29.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#54682: 29.0.50; [PATCH] Improve Eshell's logic for finding delimiters in arguments

[Jim Porter]

On 4/3/2022 5:17 AM, Lars Ingebrigtsen wrote:
> Thanks; pushed to Emacs 29.

Thanks for merging.

(Just a note for those who might be interested in this fix: there are 
still a few escaping-related bugs in Eshell that I'm working on fixes 
for. I'll try to post a patch for those after bug#54470 merges, since my 
patch builds on the patches in that bug.)