all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Jim Porter <jporterbugs@gmail.com>
To: 55204@debbugs.gnu.org
Subject: bug#55204: [PATCH] 29.0.50; Improve quoting consistency in Eshell predicates/modifiers
Date: Sat, 30 Apr 2022 21:50:42 -0700	[thread overview]
Message-ID: <b5c2a4f3-b4fb-b570-55fd-795c91a5f8f3@gmail.com> (raw)
In-Reply-To: <155d0ab6-fe59-940d-23ad-34a6d066fc46@gmail.com>

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

See the attached patches. Some comments about the patch below.

On 4/30/2022 9:37 PM, Jim Porter wrote:
> Currently, there are some inconsistencies with how string parameters 
> should be quoted in Eshell argument predicates/modifiers (hereafter just 
> "predicates"). First, the possible delimiters for a given predicate vary 
> based on which predicate is used. Currently, the allowed delimiters are:
> 
> Any non-digit character:
>    u (user)
>    g (group)
>    a (access time)
>    m (modification time)
>    c (change time)
> Any character:
>    :i (include)
>    :x (exclude)
>    :s (substitute)
> "'" or "/":
>    :j (join)
>    :S (split)

To resolve this, I've standardized on using the following delimiter 
pairs: "...", '...', /.../, |...|, (...), [...], <...>, and {...}. This 
is a smaller valid set than what's was allowed for some of the above 
predicates, but is still considerably more than what was actually 
documented (just '...' and /.../). I've updated the manual to list the 
accepted delimiters, but maybe we should add more to the list. However, 
since some predicates take an *optional* string parameter, it's best not 
to be too loose with acceptable delimiters so that there isn't 
confusion; this is already a problem with "/", since that could be a 
string delimiter *or* a predicate meaning "match directories", but it'd 
be too hard to change either of those at this point. (Users can avoid 
this issue by being careful about the order they write their predicates in.)

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.

[-- Attachment #2: 0001-Use-a-common-set-of-string-delimiters-for-all-Eshell.patch --]
[-- Type: text/plain, Size: 21010 bytes --]

From ce2fcdcd9af7db745760e4179a661d5ed186ea04 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
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).
---
 doc/misc/eshell.texi              |  12 ++
 lisp/eshell/em-pred.el            | 273 ++++++++++++++----------------
 test/lisp/eshell/em-pred-tests.el |  33 ++--
 3 files changed, 160 insertions(+), 158 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/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"))))
 
 \f
-;; 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


[-- Attachment #3: 0002-Handle-escaped-characters-in-Eshell-argument-predica.patch --]
[-- Type: text/plain, Size: 2322 bytes --]

From 4408be75ba7e7cc0e1bb7a56438f300ced3db56b Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
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


[-- Attachment #4: 0003-Handle-escaped-characters-in-Eshell-special-referenc.patch --]
[-- Type: text/plain, Size: 4249 bytes --]

From 700773e1c988da985cbbbae9b88bad29f08ae3c0 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
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


  reply	other threads:[~2022-05-01  4:50 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-01  4:37 bug#55204: 29.0.50; Improve quoting consistency in Eshell predicates/modifiers Jim Porter
2022-05-01  4:50 ` Jim Porter [this message]
2022-05-01  8:40   ` Lars Ingebrigtsen
2022-05-01 18:18     ` bug#55204: [PATCH v2] " Jim Porter
2022-05-01 18:22       ` Lars Ingebrigtsen
2022-05-01 18:36         ` Jim Porter
2022-05-01 18:40           ` Lars Ingebrigtsen
2022-05-01 18:41             ` Lars Ingebrigtsen
2022-05-01 18:42               ` Lars Ingebrigtsen
2022-05-01 23:55                 ` 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=b5c2a4f3-b4fb-b570-55fd-795c91a5f8f3@gmail.com \
    --to=jporterbugs@gmail.com \
    --cc=55204@debbugs.gnu.org \
    /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.