unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#55204: 29.0.50; Improve quoting consistency in Eshell predicates/modifiers
@ 2022-05-01  4:37 Jim Porter
  2022-05-01  4:50 ` bug#55204: [PATCH] " Jim Porter
  0 siblings, 1 reply; 10+ messages in thread
From: Jim Porter @ 2022-05-01  4:37 UTC (permalink / raw)
  To: 55204

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)

Relatedly, although the string parameters are parsed so that you can 
escape the delimiter with "\", the backslash isn't actually removed 
before using the parameter. For example, from "emacs -Q --eval '(eshell)':

   ~ $ echo $(list "foo" "bar")(:j'\'')
   foo\'bar

That should print "foo'bar" instead. Similarly, when redirecting output 
to a buffer, escapes aren't properly removed. Again, from "emacs -Q 
--eval '(eshell)'":

   ~ $ echo hi > #<foo\>bar>
   ;; Writes to the buffer "foo\>bar"

Patch forthcoming shortly (just getting a bug number first).





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#55204: [PATCH] 29.0.50; Improve quoting consistency in Eshell predicates/modifiers
  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
  2022-05-01  8:40   ` bug#55204: " Lars Ingebrigtsen
  0 siblings, 1 reply; 10+ messages in thread
From: Jim Porter @ 2022-05-01  4:50 UTC (permalink / raw)
  To: 55204

[-- 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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* bug#55204: 29.0.50; Improve quoting consistency in Eshell predicates/modifiers
  2022-05-01  4:50 ` bug#55204: [PATCH] " Jim Porter
@ 2022-05-01  8:40   ` Lars Ingebrigtsen
  2022-05-01 18:18     ` bug#55204: [PATCH v2] " Jim Porter
  0 siblings, 1 reply; 10+ messages in thread
From: Lars Ingebrigtsen @ 2022-05-01  8:40 UTC (permalink / raw)
  To: Jim Porter; +Cc: 55204

Jim Porter <jporterbugs@gmail.com> 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.

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





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#55204: [PATCH v2] 29.0.50; Improve quoting consistency in Eshell predicates/modifiers
  2022-05-01  8:40   ` bug#55204: " Lars Ingebrigtsen
@ 2022-05-01 18:18     ` Jim Porter
  2022-05-01 18:22       ` Lars Ingebrigtsen
  0 siblings, 1 reply; 10+ messages in thread
From: Jim Porter @ 2022-05-01 18:18 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 55204

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

On 5/1/2022 1:40 AM, Lars Ingebrigtsen wrote:
> Jim Porter <jporterbugs@gmail.com> 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).

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

From 3d4333af88cbc5fe206a89f7c844f757a9584fd8 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).

* 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"))))
 
 \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 6d3a9e667369643cf57b548f31bb80c45818ec80 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 d8dfe418fb51e0146c25cf8b41ab913e9ac60233 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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* bug#55204: [PATCH v2] 29.0.50; Improve quoting consistency in Eshell predicates/modifiers
  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
  0 siblings, 1 reply; 10+ messages in thread
From: Lars Ingebrigtsen @ 2022-05-01 18:22 UTC (permalink / raw)
  To: Jim Porter; +Cc: 55204

Jim Porter <jporterbugs@gmail.com> writes:

> 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).

Thanks; looks good.  But this leads to a couple of test failures, like
the one below.  (Note -- this doesn't fail when "make eshell-tests", but
with "make check".)

Test eshell-test/redirect-buffer backtrace:
  signal(ert-test-failed (((should (equal (buffer-string) "hi")) :form
  ert-fail(((should (equal (buffer-string) "hi")) :form (equal "" "hi"
  #f(compiled-function () #<bytecode 0xb0e2b90f44d2f2e>)()
  ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
  ert-run-test(#s(ert-test :name eshell-test/redirect-buffer :document
  ert-run-or-rerun-test(#s(ert--stats :selector (not ...) :tests [... 
  ert-run-tests((not (or (tag :expensive-test) (tag :unstable) (tag :n
  ert-run-tests-batch((not (or (tag :expensive-test) (tag :unstable) (
  ert-run-tests-batch-and-exit((not (or (tag :expensive-test) (tag :un
  eval((ert-run-tests-batch-and-exit '(not (or (tag :expensive-test) (
  command-line-1(("-L" ":." "-l" "ert" "-l" "lisp/eshell/eshell-tests"
  command-line()
  normal-top-level()
Test eshell-test/redirect-buffer condition:
    (ert-test-failed
     ((should
       (equal
	(buffer-string)
	"hi"))
      :form
      (equal "" "hi")
      :value nil :explanation
      (arrays-of-different-length 0 2 "" "hi" first-mismatch-at 0)))
   FAILED  19/25  eshell-test/redirect-buffer (0.001028 sec) at lisp/eshell/eshell-tests.el:117
Test eshell-test/redirect-buffer-escaped backtrace:


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





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#55204: [PATCH v2] 29.0.50; Improve quoting consistency in Eshell predicates/modifiers
  2022-05-01 18:22       ` Lars Ingebrigtsen
@ 2022-05-01 18:36         ` Jim Porter
  2022-05-01 18:40           ` Lars Ingebrigtsen
  0 siblings, 1 reply; 10+ messages in thread
From: Jim Porter @ 2022-05-01 18:36 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 55204

On 5/1/2022 11:22 AM, Lars Ingebrigtsen wrote:
> Thanks; looks good.  But this leads to a couple of test failures, like
> the one below.  (Note -- this doesn't fail when "make eshell-tests", but
> with "make check".)
> 
> Test eshell-test/redirect-buffer backtrace:
>    signal(ert-test-failed (((should (equal (buffer-string) "hi")) :form
[snip]

This looks like it might be due to the change I made in 
`with-temp-eshell' in test/lisp/eshell/eshell-tests-helpers.el. Does it 
work if you run `rm test/lisp/eshell/*.elc' first? The failure may just 
be due to stale bytecode.





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#55204: [PATCH v2] 29.0.50; Improve quoting consistency in Eshell predicates/modifiers
  2022-05-01 18:36         ` Jim Porter
@ 2022-05-01 18:40           ` Lars Ingebrigtsen
  2022-05-01 18:41             ` Lars Ingebrigtsen
  0 siblings, 1 reply; 10+ messages in thread
From: Lars Ingebrigtsen @ 2022-05-01 18:40 UTC (permalink / raw)
  To: Jim Porter; +Cc: 55204

Jim Porter <jporterbugs@gmail.com> writes:

> This looks like it might be due to the change I made in
> `with-temp-eshell' in test/lisp/eshell/eshell-tests-helpers.el. Does
> it work if you run `rm test/lisp/eshell/*.elc' first? The failure may
> just be due to stale bytecode.

I tried that, but it didn't help.

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





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#55204: [PATCH v2] 29.0.50; Improve quoting consistency in Eshell predicates/modifiers
  2022-05-01 18:40           ` Lars Ingebrigtsen
@ 2022-05-01 18:41             ` Lars Ingebrigtsen
  2022-05-01 18:42               ` Lars Ingebrigtsen
  0 siblings, 1 reply; 10+ messages in thread
From: Lars Ingebrigtsen @ 2022-05-01 18:41 UTC (permalink / raw)
  To: Jim Porter; +Cc: 55204

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Jim Porter <jporterbugs@gmail.com> writes:
>
>> This looks like it might be due to the change I made in
>> `with-temp-eshell' in test/lisp/eshell/eshell-tests-helpers.el. Does
>> it work if you run `rm test/lisp/eshell/*.elc' first? The failure may
>> just be due to stale bytecode.
>
> I tried that, but it didn't help.

Oh, sorry -- I misread.  I tried deleting the .elc files in the lisp
dir, not the test dir.  Let me try again.

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





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#55204: [PATCH v2] 29.0.50; Improve quoting consistency in Eshell predicates/modifiers
  2022-05-01 18:41             ` Lars Ingebrigtsen
@ 2022-05-01 18:42               ` Lars Ingebrigtsen
  2022-05-01 23:55                 ` Jim Porter
  0 siblings, 1 reply; 10+ messages in thread
From: Lars Ingebrigtsen @ 2022-05-01 18:42 UTC (permalink / raw)
  To: Jim Porter; +Cc: 55204

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Oh, sorry -- I misread.  I tried deleting the .elc files in the lisp
> dir, not the test dir.  Let me try again.

Yup; works fine; now pushed.

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





^ permalink raw reply	[flat|nested] 10+ messages in thread

* bug#55204: [PATCH v2] 29.0.50; Improve quoting consistency in Eshell predicates/modifiers
  2022-05-01 18:42               ` Lars Ingebrigtsen
@ 2022-05-01 23:55                 ` Jim Porter
  0 siblings, 0 replies; 10+ messages in thread
From: Jim Porter @ 2022-05-01 23:55 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 55204

On 5/1/2022 11:42 AM, Lars Ingebrigtsen wrote:
> Lars Ingebrigtsen <larsi@gnus.org> writes:
> 
>> Oh, sorry -- I misread.  I tried deleting the .elc files in the lisp
>> dir, not the test dir.  Let me try again.
> 
> Yup; works fine; now pushed.

Cool, glad that's all it needed. Thanks for merging.





^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2022-05-01 23:55 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-05-01  4:37 bug#55204: 29.0.50; Improve quoting consistency in Eshell predicates/modifiers Jim Porter
2022-05-01  4:50 ` bug#55204: [PATCH] " Jim Porter
2022-05-01  8:40   ` bug#55204: " 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

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).