Subject: Re: bug#55204: [PATCH v2] 29.0.50; Improve quoting consistency in Eshell predicates/modifiers

From: Jim Porter <[email-masked]>
Date: Sun, 1 May 2022 11:18:49 -0700

On 5/1/2022 1:40 AM, Lars Ingebrigtsen wrote:
> Jim Porter <[email-masked]> writes:
> 
>> I'm not sure if this change warrants a NEWS entry. On the one hand,
>> it's just a bug fix, but on the other hand, it's an incompatible
>> change. On the third(?) hand, it was never documented, and I'm not
>> sure if anyone would have guessed that you can use, say, alphabetic
>> characters as string delimiters in predicates.
> 
> I think it warrants a NEWS entry -- some people may have used
> undocumented values.

Ok, updated (only the first patch has any changes). Hopefully the 
wording is ok.

I've also moved a stray Eshell entry from the "Lisp Changes" section 
into the Eshell subsection of "Changes in Specialized Modes". It was 
more of a user-facing change, so it's probably best to put it there 
alongside all the other similar changes (I added a pointer to the manual 
section too).
From 3d4333af88cbc5fe206a89f7c844f757a9584fd8 Mon Sep 17 00:00:00 2001
From: Jim Porter <[email-masked]>
Date: Sun, 27 Mar 2022 22:28:40 -0700
Subject: [PATCH 1/3] Use a common set of string delimiters for all Eshell
 predicates/modifiers

* lisp/eshell/em-pred.el (eshell-pred-delimiter-pairs): New variable.
(eshell-get-comparison-modifier-argument)
(eshell-get-numeric-modifier-argument)
(eshell-get-delimited-modifier-argument): New functions...
(eshell-pred-user-or-group, eshell-pred-file-time)
(eshell-pred-file-links, eshell-pred-file-size)
(eshell-pred-substitute, eshell-join-memebers, eshell-split-members):
... and use them here.
(eshell-include-members): Pass 'mod-char' and use
'eshell-get-delimited-modifier-argument'.
(eshell-pred-file-type, eshell-pred-file-mode): Use 'when-let'.
(eshell-modifier-alist): Pass modifier char to
'eshell-include-members'.

* test/lisp/eshell/em-pred-tests.el
(em-pred-test/predicate-delimiters): New test.
(em-pred-test/predicate-uid, em-pred-test/predicate-gid,
em-pred-test/modifier-include, em-pred-test/modifier-exclude): Remove
cases covered by 'em-pred-test/predicate-delimiters'.
(em-pred-test/modifier-substitute): Add test cases for new delimiter
styles.

* doc/misc/eshell.texi (Argument Predication and Modification):
Explain how string parameters are delimited.
(Argument Modifiers): Document some special delimiter behavior with
the 's/PATTERN/REPLACE/' modifier (bug#55204).

* etc/NEWS: Announce this change, and move the
'eshell-eval-using-options' entry to the Eshell section.
---
 doc/misc/eshell.texi              |  12 ++
 etc/NEWS                          |  26 ++-
 lisp/eshell/em-pred.el            | 273 ++++++++++++++----------------
 test/lisp/eshell/em-pred-tests.el |  33 ++--
 4 files changed, 180 insertions(+), 164 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index e539206166..a3ed922cf2 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1191,6 +1191,13 @@ Argument Predication and Modification
 in the current directory and @samp{*(^@@:U^u0)} expands to all
 non-symlinks not owned by @code{root}, upper-cased.
 
+Some predicates and modifiers accept string parameters, such as
+@samp{*(u'@var{user}')}, which matches all files owned by @var{user}.
+These parameters must be surrounded by delimiters; you can use any of
+the following pairs of delimiters: @code{" @dots{} "}, @code{' @dots{}
+'}, @code{/ @dots{} /}, @code{| @dots{} |}, @code{( @dots{} )},
+@code{[ @dots{} ]}, @code{< @dots{} >}, or @code{@{ @dots{} @}}.
+
 You can customize the syntax and behavior of predicates and modifiers
 in Eshell via the Customize group ``eshell-pred'' (@pxref{Easy
 Customization, , , emacs, The GNU Emacs Manual}).
@@ -1379,6 +1386,11 @@ Argument Modifiers
 Replaces the first instance of the regular expression @var{pattern}
 with @var{replace}.  Signals an error if no match is found.
 
+As with other modifiers taking string parameters, you can use
+different delimiters to separate @var{pattern} and @var{replace}, such
+as @samp{s'@dots{}'@dots{}'}, @samp{s[@dots{}][@dots{}]}, or even
+@samp{s[@dots{}]/@dots{}/}.
+
 @item gs/@var{pattern}/@var{replace}/
 Replaces all instances of the regular expression @var{pattern} with
 @var{replace}.
diff --git a/etc/NEWS b/etc/NEWS
index 47e04cfcbe..4fdfd0b586 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -174,11 +174,22 @@ files that were compiled with an old EIEIO (Emacs<25).
 ** 'C-x 8 .' has been moved to 'C-x 8 . .'.
 This is to open up the 'C-x 8 .' map to bind further characters there.
 
+** Eshell
+
 ---
-** 'source' and '.' in Eshell no longer accept the '--help' option.
+*** 'source' and '.' no longer accept the '--help' option.
 This is for compatibility with the shell versions of these commands,
 which don't handle options like '--help' in any special way.
 
++++
+*** String delimiters in argument predicates/modifiers are more restricted.
+Previously, some argument predicates/modifiers allowed arbitrary
+characters as string delimiters.  To provide more unified behavior
+across all predicates/modifiers, the list of allowed delimiters has
+been restricted to "...", '...', /.../, |...|, (...), [...], <...>,
+and {...}.  See the "(eshell) Argument Predication and Modification"
+node in the Eshell manual for more details.
+
 ---
 ** The 'delete-forward-char' command now deletes by grapheme clusters.
 This command is by default bound to the <Delete> function key
@@ -1332,6 +1343,14 @@ Lisp function.  This frees you from having to keep track of whether
 commands are Lisp function or external when supplying absolute file
 name arguments.  See "Electric forward slash" in the Eshell manual.
 
+---
+*** Built-in Eshell commands now follow POSIX/GNU argument syntax conventions.
+Built-in commands in Eshell now accept command-line options with
+values passed as a single token, such as '-oVALUE' or
+'--option=VALUE'.  New commands can take advantage of this with the
+'eshell-eval-using-options' macro.  See "Defining new built-in
+commands" in the "(eshell) Built-ins" node of the Eshell manual.
+
 ** Calc
 
 +++
@@ -1909,11 +1928,6 @@ dimensions.
 Specifying a cons as the FROM argument allows to start measuring text
 from a specified amount of pixels above or below a position.
 
----
-** 'eshell-eval-using-options' now follows argument syntax conventions.
-Built-in commands in Eshell now accept command-line options with
-values passed as a single token, such as '-oVALUE' or '--option=VALUE'.
-
 ** XDG support
 
 ---
diff --git a/lisp/eshell/em-pred.el b/lisp/eshell/em-pred.el
index eb5109b82d..594563554d 100644
--- a/lisp/eshell/em-pred.el
+++ b/lisp/eshell/em-pred.el
@@ -116,8 +116,8 @@ eshell-modifier-alist
     (?U . (lambda (lst) (mapcar #'upcase lst)))
     (?C . (lambda (lst) (mapcar #'capitalize lst)))
     (?h . (lambda (lst) (mapcar #'file-name-directory lst)))
-    (?i . (eshell-include-members))
-    (?x . (eshell-include-members t))
+    (?i . (eshell-include-members ?i))
+    (?x . (eshell-include-members ?x t))
     (?r . (lambda (lst) (mapcar #'file-name-sans-extension lst)))
     (?e . (lambda (lst) (mapcar #'file-name-extension lst)))
     (?t . (lambda (lst) (mapcar #'file-name-nondirectory lst)))
@@ -219,6 +219,20 @@ eshell-modifier-help-string
 EXAMPLES:
   *.c(:o)  sorted list of .c files")
 
+(defvar eshell-pred-delimiter-pairs
+  '((?\( . ?\))
+    (?\[ . ?\])
+    (?\< . ?\>)
+    (?\{ . ?\})
+    (?\' . ?\')
+    (?\" . ?\")
+    (?/  . ?/)
+    (?|  . ?|))
+  "A list of delimiter pairs that can be used in argument predicates/modifiers.
+Each element is of the form (OPEN . CLOSE), where OPEN and CLOSE
+are characters representing the opening and closing delimiter,
+respectively.")
+
 (defvar-keymap eshell-pred-mode-map
   "C-c M-q" #'eshell-display-predicate-help
   "C-c M-m" #'eshell-display-modifier-help)
@@ -364,38 +378,68 @@ eshell-add-pred-func
                  (lambda (file) (funcall pred (file-truename file))))))
   (cons pred funcs))
 
+(defun eshell-get-comparison-modifier-argument (&optional functions)
+  "Starting at point, get the comparison modifier argument, if any.
+These are the -/+ characters, corresponding to `<' and `>',
+respectively.  If no comparison modifier is at point, return `='.
+
+FUNCTIONS, if non-nil, is a list of comparison functions,
+specified as (LESS-THAN GREATER-THAN EQUAL-TO)."
+  (let ((functions (or functions (list #'< #'> #'=))))
+    (if (memq (char-after) '(?- ?+))
+        (prog1
+            (if (eq (char-after) ?-) (nth 0 functions) (nth 1 functions))
+          (forward-char))
+      (nth 2 functions))))
+
+(defun eshell-get-numeric-modifier-argument ()
+  "Starting at point, get the numeric modifier argument, if any.
+If a number is found, update point to just after the number."
+  (when (looking-at "[0-9]+")
+    (prog1
+	(string-to-number (match-string 0))
+      (goto-char (match-end 0)))))
+
+(defun eshell-get-delimited-modifier-argument (&optional chained-p)
+  "Starting at point, get the delimited modifier argument, if any.
+If the character after point is a predicate/modifier
+delimiter (see `eshell-pred-delimiter-pairs', read the value of
+the argument and update point to be just after the closing
+delimiter.
+
+If CHAINED-P is true, then another delimited modifier argument
+will immediately follow this one.  In this case, when the opening
+and closing delimiters are the same, update point to be just
+before the closing delimiter. This allows modifiers like
+`:s/match/repl' to work as expected."
+  (when-let* ((open (char-after))
+              (close (cdr (assoc open eshell-pred-delimiter-pairs)))
+              (end (eshell-find-delimiter open close nil nil t)))
+    (prog1
+        (buffer-substring-no-properties (1+ (point)) end)
+      (goto-char (if (and chained-p (eq open close))
+                     end
+                   (1+ end))))))
+
 (defun eshell-pred-user-or-group (mod-char mod-type attr-index get-id-func)
   "Return a predicate to test whether a file match a given user/group id."
-  (let (ugid open close end)
-    (if (looking-at "[0-9]+")
-	(progn
-	  (setq ugid (string-to-number (match-string 0)))
-	  (goto-char (match-end 0)))
-      (setq open (char-after))
-      (if (setq close (memq open '(?\( ?\[ ?\< ?\{)))
-	  (setq close (car (last '(?\) ?\] ?\> ?\})
-				 (length close))))
-	(setq close open))
-      (forward-char)
-      (setq end (eshell-find-delimiter open close))
-      (unless end
-	(error "Malformed %s name string for modifier `%c'"
-	       mod-type mod-char))
-      (setq ugid
-	    (funcall get-id-func (buffer-substring (point) end)))
-      (goto-char (1+ end)))
+  (let ((ugid (eshell-get-numeric-modifier-argument)))
+    (unless ugid
+      (let ((ugname (or (eshell-get-delimited-modifier-argument)
+                        (error "Malformed %s name string for modifier `%c'"
+                               mod-type mod-char))))
+        (setq ugid (funcall get-id-func ugname))))
     (unless ugid
       (error "Unknown %s name specified for modifier `%c'"
 	     mod-type mod-char))
     (lambda (file)
-      (let ((attrs (file-attributes file)))
-	(if attrs
-	    (= (nth attr-index attrs) ugid))))))
+      (when-let ((attrs (file-attributes file)))
+	(= (nth attr-index attrs) ugid)))))
 
 (defun eshell-pred-file-time (mod-char mod-type attr-index)
   "Return a predicate to test whether a file matches a certain time."
   (let* ((quantum 86400)
-	 qual when open close end)
+	 qual when)
     (when (memq (char-after) '(?M ?w ?h ?m ?s))
       (setq quantum (char-after))
       (cond
@@ -410,36 +454,21 @@ eshell-pred-file-time
        ((eq quantum ?s)
 	(setq quantum 1)))
       (forward-char))
-    (when (memq (char-after) '(?+ ?-))
-      (setq qual (char-after))
-      (forward-char))
-    (if (looking-at "[0-9]+")
-	(progn
-	  (setq when (time-since (* (string-to-number (match-string 0))
-				    quantum)))
-	  (goto-char (match-end 0)))
-      (setq open (char-after))
-      (if (setq close (memq open '(?\( ?\[ ?\< ?\{)))
-	  (setq close (car (last '(?\) ?\] ?\> ?\})
-				 (length close))))
-	(setq close open))
-      (forward-char)
-      (setq end (eshell-find-delimiter open close))
-      (unless end
-	(error "Malformed %s time modifier `%c'" mod-type mod-char))
-      (let* ((file (buffer-substring (point) end))
-	     (attrs (file-attributes file)))
-	(unless attrs
-	  (error "Cannot stat file `%s'" file))
-	(setq when (nth attr-index attrs)))
-      (goto-char (1+ end)))
-    (let ((f (cond ((eq qual ?-) #'time-less-p)
-                     ((eq qual ?+) (lambda (a b) (time-less-p b a)))
-                     (#'time-equal-p))))
-      (lambda (file)
-	(let ((attrs (file-attributes file)))
-	  (if attrs
-              (funcall f when (nth attr-index attrs))))))))
+    (setq qual (eshell-get-comparison-modifier-argument
+                (list #'time-less-p
+                      (lambda (a b) (time-less-p b a))
+                      #'time-equal-p)))
+    (if-let ((number (eshell-get-numeric-modifier-argument)))
+        (setq when (time-since (* number quantum)))
+      (let* ((file (or (eshell-get-delimited-modifier-argument)
+                       (error "Malformed %s time modifier `%c'"
+                              mod-type mod-char)))
+             (attrs (or (file-attributes file)
+                        (error "Cannot stat file `%s'" file))))
+        (setq when (nth attr-index attrs))))
+    (lambda (file)
+      (when-let ((attrs (file-attributes file)))
+        (funcall qual when (nth attr-index attrs))))))
 
 (defun eshell-pred-file-type (type)
   "Return a test which tests that the file is of a certain TYPE.
@@ -454,36 +483,23 @@ eshell-pred-file-type
 		 '(?b ?c)
 	       (list type))))
     (lambda (file)
-      (let ((attrs (eshell-file-attributes (directory-file-name file))))
-	(if attrs
-	    (memq (aref (file-attribute-modes attrs) 0) set))))))
+      (when-let ((attrs (eshell-file-attributes (directory-file-name file))))
+	(memq (aref (file-attribute-modes attrs) 0) set)))))
 
 (defsubst eshell-pred-file-mode (mode)
   "Return a test which tests that MODE pertains to the file."
   (lambda (file)
-    (let ((modes (file-modes file 'nofollow)))
-      (if modes
-	  (not (zerop (logand mode modes)))))))
+    (when-let ((modes (file-modes file 'nofollow)))
+      (not (zerop (logand mode modes))))))
 
 (defun eshell-pred-file-links ()
   "Return a predicate to test whether a file has a given number of links."
-  (let (qual amount)
-    (when (memq (char-after) '(?- ?+))
-      (setq qual (char-after))
-      (forward-char))
-    (unless (looking-at "[0-9]+")
-      (error "Invalid file link count modifier `l'"))
-    (setq amount (string-to-number (match-string 0)))
-    (goto-char (match-end 0))
-    (let ((f (if (eq qual ?-)
-		 #'<
-	       (if (eq qual ?+)
-		   #'>
-		 #'=))))
-      (lambda (file)
-	(let ((attrs (eshell-file-attributes file)))
-	  (if attrs
-	      (funcall f (file-attribute-link-number attrs) amount)))))))
+  (let ((qual (eshell-get-comparison-modifier-argument))
+        (amount (or (eshell-get-numeric-modifier-argument)
+                    (error "Invalid file link count modifier `l'"))))
+    (lambda (file)
+      (when-let ((attrs (eshell-file-attributes file)))
+	  (funcall qual (file-attribute-link-number attrs) amount)))))
 
 (defun eshell-pred-file-size ()
   "Return a predicate to test whether a file is of a given size."
@@ -498,85 +514,52 @@ eshell-pred-file-size
        ((eq qual ?p)
 	(setq quantum 512)))
       (forward-char))
-    (when (memq (char-after) '(?- ?+))
-      (setq qual (char-after))
-      (forward-char))
-    (unless (looking-at "[0-9]+")
-      (error "Invalid file size modifier `L'"))
-    (setq amount (* (string-to-number (match-string 0)) quantum))
-    (goto-char (match-end 0))
-    (let ((f (if (eq qual ?-)
-		 #'<
-	       (if (eq qual ?+)
-		   #'>
-		 #'=))))
-      (lambda (file)
-	(let ((attrs (eshell-file-attributes file)))
-	  (if attrs
-	      (funcall f (file-attribute-size attrs) amount)))))))
+    (setq qual (eshell-get-comparison-modifier-argument))
+    (setq amount (* (or (eshell-get-numeric-modifier-argument)
+                        (error "Invalid file size modifier `L'"))
+                    quantum))
+    (lambda (file)
+      (when-let ((attrs (eshell-file-attributes file)))
+	(funcall qual (file-attribute-size attrs) amount)))))
 
 (defun eshell-pred-substitute (&optional repeat)
   "Return a modifier function that will substitute matches."
-  (let ((delim (char-after))
-	match replace end)
-    (forward-char)
-    (setq end (eshell-find-delimiter delim delim nil nil t)
-	  match (buffer-substring-no-properties (point) end))
-    (goto-char (1+ end))
-    (setq end (eshell-find-delimiter delim delim nil nil t)
-	  replace (buffer-substring-no-properties (point) end))
-    (goto-char (1+ end))
-    (if repeat
-	(lambda (lst)
-	  (mapcar
-           (lambda (str)
-             (replace-regexp-in-string match replace str t))
-           lst))
-      (lambda (lst)
-	(mapcar
-         (lambda (str)
-           (if (string-match match str)
-               (replace-match replace t nil str)
-             (error (concat str ": substitution failed"))))
-         lst)))))
-
-(defun eshell-include-members (&optional invert-p)
-  "Include only Lisp members matching a regexp."
-  (let ((delim (char-after))
-	regexp end)
-    (forward-char)
-    (setq end (eshell-find-delimiter delim delim nil nil t)
-	  regexp (buffer-substring-no-properties (point) end))
-    (goto-char (1+ end))
-    (let ((predicates
-	   (list (if invert-p
-		     (lambda (elem) (not (string-match regexp elem)))
-		   (lambda (elem) (string-match regexp elem))))))
-      (lambda (lst)
-	(eshell-winnow-list lst nil predicates)))))
+  (let* ((match (or (eshell-get-delimited-modifier-argument t)
+                    (error "Malformed pattern string for modifier `s'")))
+         (replace (or (eshell-get-delimited-modifier-argument)
+                      (error "Malformed replace string for modifier `s'")))
+         (function (if repeat
+                       (lambda (str)
+                         (replace-regexp-in-string match replace str t))
+                     (lambda (str)
+                       (if (string-match match str)
+                           (replace-match replace t nil str)
+                         (error (concat str ": substitution failed")))))))
+    (lambda (lst) (mapcar function lst))))
+
+(defun eshell-include-members (mod-char &optional invert-p)
+  "Include only Lisp members matching a regexp.
+If INVERT-P is non-nil, include only members not matching a regexp."
+  (let* ((regexp (or (eshell-get-delimited-modifier-argument)
+                     (error "Malformed pattern string for modifier `%c'"
+                            mod-char)))
+         (predicates
+	  (list (if invert-p
+		    (lambda (elem) (not (string-match regexp elem)))
+		  (lambda (elem) (string-match regexp elem))))))
+    (lambda (lst)
+      (eshell-winnow-list lst nil predicates))))
 
 (defun eshell-join-members ()
   "Return a modifier function that join matches."
-  (let ((delim (char-after))
-	str end)
-    (if (not (memq delim '(?' ?/)))
-	(setq str " ")
-      (forward-char)
-      (setq end (eshell-find-delimiter delim delim nil nil t)
-	    str (buffer-substring-no-properties (point) end))
-      (goto-char (1+ end)))
+  (let ((str (or (eshell-get-delimited-modifier-argument)
+                 " ")))
     (lambda (lst)
       (mapconcat #'identity lst str))))
 
 (defun eshell-split-members ()
   "Return a modifier function that splits members."
-  (let ((delim (char-after))
-	sep end)
-    (when (memq delim '(?' ?/))
-      (forward-char)
-      (setq end (eshell-find-delimiter delim delim nil nil t)
-	    sep (buffer-substring-no-properties (point) end))
-      (goto-char (1+ end)))
+  (let ((sep (eshell-get-delimited-modifier-argument)))
     (lambda (lst)
       (mapcar
        (lambda (str)
diff --git a/test/lisp/eshell/em-pred-tests.el b/test/lisp/eshell/em-pred-tests.el
index 7f88ac4475..4d2af39292 100644
--- a/test/lisp/eshell/em-pred-tests.el
+++ b/test/lisp/eshell/em-pred-tests.el
@@ -26,6 +26,7 @@
 (require 'ert)
 (require 'esh-mode)
 (require 'eshell)
+(require 'em-pred)
 
 (require 'eshell-tests-helpers
          (expand-file-name "eshell-tests-helpers"
@@ -254,8 +255,6 @@ em-pred-test/predicate-uid
       (cl-letf (((symbol-function 'eshell-user-id)
                  (lambda (name) (seq-position user-names name))))
         (should (equal (eshell-eval-predicate files "u'one'")
-                       '("/fake/uid=1")))
-        (should (equal (eshell-eval-predicate files "u{one}")
                        '("/fake/uid=1")))))))
 
 (ert-deftest em-pred-test/predicate-gid ()
@@ -268,8 +267,6 @@ em-pred-test/predicate-gid
       (cl-letf (((symbol-function 'eshell-group-id)
                  (lambda (name) (seq-position group-names name))))
         (should (equal (eshell-eval-predicate files "g'one'")
-                       '("/fake/gid=1")))
-        (should (equal (eshell-eval-predicate files "g{one}")
                        '("/fake/gid=1")))))))
 
 (defmacro em-pred-test--time-deftest (name file-attribute predicate
@@ -430,6 +427,8 @@ em-pred-test/modifier-substitute
   "Test that \":s/PAT/REP/\" replaces PAT with REP once."
   (should (equal (eshell-eval-predicate "bar" ":s/a/*/") "b*r"))
   (should (equal (eshell-eval-predicate "bar" ":s|a|*|") "b*r"))
+  (should (equal (eshell-eval-predicate "bar" ":s{a}{*}") "b*r"))
+  (should (equal (eshell-eval-predicate "bar" ":s{a}'*'") "b*r"))
   (should (equal (eshell-eval-predicate '("foo" "bar" "baz") ":s/[ao]/*/")
                  '("f*o" "b*r" "b*z")))
   (should (equal (eshell-eval-predicate '("foo" "bar" "baz") ":s|[ao]|*|")
@@ -450,23 +449,15 @@ em-pred-test/modifier-global-substitute
 (ert-deftest em-pred-test/modifier-include ()
   "Test that \":i/PAT/\" filters elements to include only ones matching PAT."
   (should (equal (eshell-eval-predicate "foo" ":i/a/") nil))
-  (should (equal (eshell-eval-predicate "foo" ":i|a|") nil))
   (should (equal (eshell-eval-predicate "bar" ":i/a/") "bar"))
-  (should (equal (eshell-eval-predicate "bar" ":i|a|") "bar"))
   (should (equal (eshell-eval-predicate '("foo" "bar" "baz") ":i/a/")
-                 '("bar" "baz")))
-  (should (equal (eshell-eval-predicate '("foo" "bar" "baz") ":i|a|")
                  '("bar" "baz"))))
 
 (ert-deftest em-pred-test/modifier-exclude ()
   "Test that \":x/PAT/\" filters elements to exclude any matching PAT."
   (should (equal (eshell-eval-predicate "foo" ":x/a/") "foo"))
-  (should (equal (eshell-eval-predicate "foo" ":x|a|") "foo"))
   (should (equal (eshell-eval-predicate "bar" ":x/a/") nil))
-  (should (equal (eshell-eval-predicate "bar" ":x|a|") nil))
   (should (equal (eshell-eval-predicate '("foo" "bar" "baz") ":x/a/")
-                 '("foo")))
-  (should (equal (eshell-eval-predicate '("foo" "bar" "baz") ":x|a|")
                  '("foo"))))
 
 (ert-deftest em-pred-test/modifier-split ()
@@ -516,7 +507,7 @@ em-pred-test/modifier-reverse
                  '("baz" "bar" "foo"))))
 
 
-;; Combinations
+;; Miscellaneous
 
 (ert-deftest em-pred-test/combine-predicate-and-modifier ()
   "Test combination of predicates and modifiers."
@@ -526,4 +517,20 @@ em-pred-test/combine-predicate-and-modifier
       (should (equal (eshell-eval-predicate files ".:e:u")
                      '("el" "txt"))))))
 
+(ert-deftest em-pred-test/predicate-delimiters ()
+  "Test various delimiter pairs with predicates and modifiers."
+  (dolist (delims eshell-pred-delimiter-pairs)
+    (eshell-with-file-attributes-from-name
+     (let ((files '("/fake/uid=1" "/fake/uid=2"))
+           (user-names '("root" "one" "two")))
+       (cl-letf (((symbol-function 'eshell-user-id)
+                  (lambda (name) (seq-position user-names name))))
+         (should (equal (eshell-eval-predicate
+                         files (format "u%cone%c" (car delims) (cdr delims)))
+                        '("/fake/uid=1"))))))
+    (should (equal (eshell-eval-predicate
+                    '("foo" "bar" "baz")
+                    (format ":j%c-%c" (car delims) (cdr delims)))
+                   "foo-bar-baz"))))
+
 ;; em-pred-tests.el ends here
-- 
2.25.1

From 6d3a9e667369643cf57b548f31bb80c45818ec80 Mon Sep 17 00:00:00 2001
From: Jim Porter <[email-masked]>
Date: Tue, 26 Apr 2022 21:51:23 -0700
Subject: [PATCH 2/3] Handle escaped characters in Eshell argument
 predicates/modifiers

* lisp/eshell/em-pred.el (eshell-get-delimited-modifier-argument):
Unescape escaped characters.

* test/lisp/eshell/em-pred-tests.el (em-pred-test/predicate-escaping):
New test (bug#55204).
---
 lisp/eshell/em-pred.el            |  4 +++-
 test/lisp/eshell/em-pred-tests.el | 12 ++++++++++++
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/lisp/eshell/em-pred.el b/lisp/eshell/em-pred.el
index 594563554d..d73976d346 100644
--- a/lisp/eshell/em-pred.el
+++ b/lisp/eshell/em-pred.el
@@ -416,7 +416,9 @@ eshell-get-delimited-modifier-argument
               (close (cdr (assoc open eshell-pred-delimiter-pairs)))
               (end (eshell-find-delimiter open close nil nil t)))
     (prog1
-        (buffer-substring-no-properties (1+ (point)) end)
+        (replace-regexp-in-string
+         (rx-to-string `(seq "\\" (group (or "\\" ,open ,close)))) "\\1"
+         (buffer-substring-no-properties (1+ (point)) end))
       (goto-char (if (and chained-p (eq open close))
                      end
                    (1+ end))))))
diff --git a/test/lisp/eshell/em-pred-tests.el b/test/lisp/eshell/em-pred-tests.el
index 4d2af39292..3b50543d69 100644
--- a/test/lisp/eshell/em-pred-tests.el
+++ b/test/lisp/eshell/em-pred-tests.el
@@ -533,4 +533,16 @@ em-pred-test/predicate-delimiters
                     (format ":j%c-%c" (car delims) (cdr delims)))
                    "foo-bar-baz"))))
 
+(ert-deftest em-pred-test/predicate-escaping ()
+  "Test string escaping in predicate and modifier parameters."
+  ;; Escaping the delimiter should remove the backslash.
+  (should (equal (eshell-eval-predicate '("foo" "bar" "baz") ":j'\\''")
+                 "foo'bar'baz"))
+  ;; Escaping a backlash should remove the first backslash.
+  (should (equal (eshell-eval-predicate '("foo" "bar" "baz") ":j'\\\\'")
+                 "foo\\bar\\baz"))
+  ;; Escaping a different character should keep the backslash.
+  (should (equal (eshell-eval-predicate '("foo" "bar" "baz") ":j'\\\"'")
+                 "foo\\\"bar\\\"baz")))
+
 ;; em-pred-tests.el ends here
-- 
2.25.1

From d8dfe418fb51e0146c25cf8b41ab913e9ac60233 Mon Sep 17 00:00:00 2001
From: Jim Porter <[email-masked]>
Date: Tue, 26 Apr 2022 21:53:00 -0700
Subject: [PATCH 3/3] Handle escaped characters in Eshell special references
 (e.g. buffers)

* lisp/eshell/esh-arg.el (eshell-parse-special-reference): Unescape
escaped characters.

* test/lisp/eshell/eshell-tests-helpers.el (with-temp-eshell): Restore
current buffer after evaluating BODY.

* test/lisp/eshell/eshell-tests.el (eshell-test/redirect-buffer)
(eshell-test/redirect-buffer-escaped): New tests (bug#55204).
---
 lisp/eshell/esh-arg.el                   |  4 +++-
 test/lisp/eshell/eshell-tests-helpers.el | 23 ++++++++++++-----------
 test/lisp/eshell/eshell-tests.el         | 19 +++++++++++++++++++
 3 files changed, 34 insertions(+), 12 deletions(-)

diff --git a/lisp/eshell/esh-arg.el b/lisp/eshell/esh-arg.el
index ee3f907f85..395aa87ff0 100644
--- a/lisp/eshell/esh-arg.el
+++ b/lisp/eshell/esh-arg.el
@@ -401,7 +401,9 @@ eshell-parse-special-reference
         (if (eshell-arg-delimiter (1+ end))
             (prog1
                 (list (if buffer-p 'get-buffer-create 'get-process)
-                      (buffer-substring-no-properties (point) end))
+                      (replace-regexp-in-string
+                       (rx "\\" (group (or "\\" "<" ">"))) "\\1"
+                       (buffer-substring-no-properties (point) end)))
               (goto-char (1+ end)))
           (ignore (goto-char here)))))))
 
diff --git a/test/lisp/eshell/eshell-tests-helpers.el b/test/lisp/eshell/eshell-tests-helpers.el
index f944194a2b..4ad76ca697 100644
--- a/test/lisp/eshell/eshell-tests-helpers.el
+++ b/test/lisp/eshell/eshell-tests-helpers.el
@@ -38,17 +38,18 @@ eshell-test--max-subprocess-time
 
 (defmacro with-temp-eshell (&rest body)
   "Evaluate BODY in a temporary Eshell buffer."
-  `(ert-with-temp-directory eshell-directory-name
-     (let* (;; We want no history file, so prevent Eshell from falling
-            ;; back on $HISTFILE.
-            (process-environment (cons "HISTFILE" process-environment))
-            (eshell-history-file-name nil)
-            (eshell-buffer (eshell t)))
-       (unwind-protect
-           (with-current-buffer eshell-buffer
-             ,@body)
-         (let (kill-buffer-query-functions)
-           (kill-buffer eshell-buffer))))))
+  `(save-current-buffer
+     (ert-with-temp-directory eshell-directory-name
+       (let* (;; We want no history file, so prevent Eshell from falling
+              ;; back on $HISTFILE.
+              (process-environment (cons "HISTFILE" process-environment))
+              (eshell-history-file-name nil)
+              (eshell-buffer (eshell t)))
+         (unwind-protect
+             (with-current-buffer eshell-buffer
+               ,@body)
+           (let (kill-buffer-query-functions)
+             (kill-buffer eshell-buffer)))))))
 
 (defun eshell-wait-for-subprocess (&optional all)
   "Wait until there is no interactive subprocess running in Eshell.
diff --git a/test/lisp/eshell/eshell-tests.el b/test/lisp/eshell/eshell-tests.el
index bcc2dc320b..7cdeb017e4 100644
--- a/test/lisp/eshell/eshell-tests.el
+++ b/test/lisp/eshell/eshell-tests.el
@@ -114,6 +114,25 @@ eshell-test/pipe-headproc-stdin
    (eshell-wait-for-subprocess)
    (eshell-match-result "OLLEH\n")))
 
+(ert-deftest eshell-test/redirect-buffer ()
+  "Check that piping to a buffer works"
+  (with-temp-buffer
+    (rename-buffer "eshell-temp-buffer" t)
+    (let ((bufname (buffer-name)))
+      (with-temp-eshell
+       (eshell-insert-command (format "echo hi > #<%s>" bufname)))
+      (should (equal (buffer-string) "hi")))))
+
+(ert-deftest eshell-test/redirect-buffer-escaped ()
+  "Check that piping to a buffer with escaped characters works"
+  (with-temp-buffer
+    (rename-buffer "eshell\\temp\\buffer" t)
+    (let ((bufname (buffer-name)))
+      (with-temp-eshell
+       (eshell-insert-command (format "echo hi > #<%s>"
+                                      (string-replace "\\" "\\\\" bufname))))
+      (should (equal (buffer-string) "hi")))))
+
 (ert-deftest eshell-test/inside-emacs-var ()
   "Test presence of \"INSIDE_EMACS\" in subprocesses"
   (with-temp-eshell
-- 
2.25.1