bug#54470: 29.0.50; [PATCH] Add documentation/tests for Eshell argument expansion

[Jim Porter <jporterbugs@gmail.com>]

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

On 3/20/2022 1:57 PM, Jim Porter wrote:
> On 3/20/2022 12:05 AM, Eli Zaretskii wrote:
>> Thank you for working on this.  See some minor comments below.

I found an additional bug in the global substitution modifier which 
could cause an infinite loop in some cases, e.g. with "(:gs/a/a/)". The 
regexp search wouldn't advance to the next character correctly and could 
get stuck. I've fixed this by using `replace-regexp-in-string' instead.

[-- Attachment #2: 0001-Add-unit-tests-and-documentation-for-Eshell-pattern-.patch --]
[-- Type: text/plain, Size: 14625 bytes --]

From fe2dda98b60f687ff4aee20b44ef000d65b16186 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Tue, 8 Mar 2022 17:07:26 -0800
Subject: [PATCH 1/3] Add unit tests and documentation for Eshell pattern-based
 globs

* lisp/eshell/em-glob.el (eshell-extended-glob): Fix docstring.
(eshell-glob-entries): Refer to '**/' in error (technically, '**' can
end a glob, but it means the same thing as '*').

* test/lisp/eshell/em-glob-tests.el: New file.

* doc/misc/eshell.texi (Globbing): Document pattern-based globs.
---
 doc/misc/eshell.texi              |  94 ++++++++++++++--
 lisp/eshell/em-glob.el            |  14 ++-
 test/lisp/eshell/em-glob-tests.el | 171 ++++++++++++++++++++++++++++++
 3 files changed, 262 insertions(+), 17 deletions(-)
 create mode 100644 test/lisp/eshell/em-glob-tests.el

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 372e4c3ffb..0b52c73176 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1089,15 +1089,91 @@ Dollars Expansion
 
 @node Globbing
 @section Globbing
-Eshell's globbing syntax is very similar to that of Zsh.  Users coming
-from Bash can still use Bash-style globbing, as there are no
-incompatibilities.  Most globbing is pattern-based expansion, but there
-is also predicate-based expansion.  @xref{Filename Generation, , ,
-zsh, The Z Shell Manual},
-for full syntax.  To customize the syntax and behavior of globbing in
-Eshell see the Customize@footnote{@xref{Easy Customization, , , emacs,
-The GNU Emacs Manual}.}
-groups ``eshell-glob'' and ``eshell-pred''.
+@vindex eshell-glob-case-insensitive
+Eshell's globbing syntax is very similar to that of Zsh
+(@pxref{Filename Generation, , , zsh, The Z Shell Manual}).  Users
+coming from Bash can still use Bash-style globbing, as there are no
+incompatibilities.
+
+By default, globs are case sensitive, except on MS-DOS/MS-Windows
+systems.  You can control this behavior via the
+@code{eshell-glob-case-insensitive} option.  You can further customize
+the syntax and behavior of globbing in Eshell via the Customize group
+``eshell-glob'' (@pxref{Easy Customization, , , emacs, The GNU Emacs
+Manual}).
+
+@table @samp
+
+@item *
+Matches any string (including the empty string).  For example,
+@samp{*.el} matches any file with the @file{.el} extension.
+
+@item ?
+Matches any single character.  For example, @samp{?at} matches
+@file{cat} and @file{bat}, but not @file{goat}.
+
+@item **/
+Matches zero or more subdirectories in a file name.  For example,
+@samp{**/foo.el} matches @file{foo.el}, @file{bar/foo.el},
+@file{bar/baz/foo.el}, etc.  Note that this cannot be combined with
+any other patterns in the same file name segment, so while
+@samp{foo/**/bar.el} is allowed, @samp{foo**/bar.el} is not.
+
+@item ***/
+Like @code{**/}, but follows symlinks as well.
+
+@cindex character sets, in Eshell glob patterns
+@cindex character classes, in Eshell glob patterns
+@item [ @dots{} ]
+Defines a @dfn{character set} (@pxref{Regexps, , , emacs, The GNU
+Emacs Manual}).  A character set matches characters between the two
+brackets; for example, @samp{[ad]} matches @file{a} and @file{d}.  You
+can also include ranges of characters in the set by separating the
+start and end with @samp{-}.  Thus, @samp{[a-z]} matches any
+lower-case @acronym{ASCII} letter.  Note that, unlike in Zsh,
+character ranges are interpreted in the Unicode codepoint order, not
+in the locale-dependent collation order.
+
+Additionally, you can include @dfn{character classes} in a character
+set.  A @samp{[:} and balancing @samp{:]} enclose a character class
+inside a character set.  For instance, @samp{[[:alnum:]]}
+matches any letter or digit.  @xref{Char Classes, , , elisp, The Emacs
+Lisp Reference Manual}, for a list of character classes.
+
+@cindex complemented character sets, in Eshell glob patterns
+@item [^ @dots{} ]
+Defines a @dfn{complemented character set}.  This behaves just like a
+character set, but matches any character @emph{except} the ones
+specified.
+
+@cindex groups, in Eshell glob patterns
+@item ( @dots{} )
+Defines a @dfn{group}.  A group matches the pattern between @samp{(}
+and @samp{)}.  Note that a group can only match a single file name
+component, so a @samp{/} inside a group will signal an error.
+
+@item @var{x}|@var{y}
+Inside of a group, matches either @var{x} or @var{y}.  For example,
+@samp{e(m|sh)-*} matches any file beginning with @code{em-} or
+@code{esh-}.
+
+@item @var{x}#
+Matches zero or more copies of the glob pattern @var{x}.  For example,
+@samp{fo#.el} matches @file{f.el}, @file{fo.el}, @file{foo.el}, etc.
+
+@item @var{x}##
+Matches one or more copies of the glob pattern @var{x}.  Thus,
+@samp{fo#.el} matches @file{fo.el}, @file{foo.el}, @file{fooo.el},
+etc.
+
+@item @var{x}~@var{y}
+Matches anything that matches the pattern @var{x} but not @var{y}. For
+example, @samp{[[:digit:]]#~4?} matches @file{1} and @file{12}, but
+not @file{42}.  Note that unlike in Zsh, only a single @code{~}
+operator can be used in a pattern, and it cannot be inside of a group
+like @samp{(@var{x}~@var{y})}.
+
+@end table
 
 @node Input/Output
 @chapter Input/Output
diff --git a/lisp/eshell/em-glob.el b/lisp/eshell/em-glob.el
index 842f27a492..52531ff893 100644
--- a/lisp/eshell/em-glob.el
+++ b/lisp/eshell/em-glob.el
@@ -233,7 +233,10 @@ eshell-glob-regexp
 	    "\\'")))
 
 (defun eshell-extended-glob (glob)
-  "Return a list of files generated from GLOB, perhaps looking for DIRS-ONLY.
+  "Return a list of files matched by GLOB.
+If no files match, signal an error (if `eshell-error-if-no-glob'
+is non-nil), or otherwise return GLOB itself.
+
 This function almost fully supports zsh style filename generation
 syntax.  Things that are not supported are:
 
@@ -243,12 +246,7 @@ eshell-extended-glob
    foo~x(a|b)  (a|b) will be interpreted as a predicate/modifier list
 
 Mainly they are not supported because file matching is done with Emacs
-regular expressions, and these cannot support the above constructs.
-
-If this routine fails, it returns nil.  Otherwise, it returns a list
-the form:
-
-   (INCLUDE-REGEXP EXCLUDE-REGEXP (PRED-FUNC-LIST) (MOD-FUNC-LIST))"
+regular expressions, and these cannot support the above constructs."
   (let ((paths (eshell-split-path glob))
         eshell-glob-matches message-shown)
     (unwind-protect
@@ -287,7 +285,7 @@ eshell-glob-entries
 		   glob (car globs)
 		   len (length glob)))))
     (if (and recurse-p (not glob))
-	(error "`**' cannot end a globbing pattern"))
+	(error "`**/' cannot end a globbing pattern"))
     (let ((index 1))
       (setq incl glob)
       (while (and (eq incl glob)
diff --git a/test/lisp/eshell/em-glob-tests.el b/test/lisp/eshell/em-glob-tests.el
new file mode 100644
index 0000000000..9976b32ffe
--- /dev/null
+++ b/test/lisp/eshell/em-glob-tests.el
@@ -0,0 +1,171 @@
+;;; em-glob-tests.el --- em-glob test suite  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2022 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; Tests for Eshell's glob expansion.
+
+;;; Code:
+
+(require 'ert)
+(require 'em-glob)
+
+(defmacro with-fake-files (files &rest body)
+  "Evaluate BODY forms, pretending that FILES exist on the filesystem.
+FILES is a list of file names that should be reported as
+appropriate by `file-name-all-completions'.  Any file name
+component ending in \"symlink\" is treated as a symbolic link."
+  (declare (indent 1))
+  `(cl-letf (((symbol-function 'file-name-all-completions)
+              (lambda (file directory)
+                (cl-assert (string= file ""))
+                (setq directory (expand-file-name directory))
+                `("./" "../"
+                  ,@(delete-dups
+                     (remq nil
+                           (mapcar
+                            (lambda (file)
+                              (setq file (expand-file-name file))
+                              (when (string-prefix-p directory file)
+                                (replace-regexp-in-string
+                                 "/.*" "/"
+                                 (substring file (length directory)))))
+                            ,files))))))
+             ((symbol-function 'file-symlink-p)
+              (lambda (file)
+                (string-suffix-p "symlink" file))))
+     ,@body))
+
+;;; Tests:
+
+(ert-deftest em-glob-test/match-any-string ()
+  "Test that \"*\" pattern matches any string."
+  (with-fake-files '("a.el" "b.el" "c.txt" "dir/a.el")
+    (should (equal (eshell-extended-glob "*.el")
+                   '("a.el" "b.el")))))
+
+(ert-deftest em-glob-test/match-any-character ()
+  "Test that \"?\" pattern matches any character."
+  (with-fake-files '("a.el" "b.el" "ccc.el" "d.txt" "dir/a.el")
+    (should (equal (eshell-extended-glob "?.el")
+                   '("a.el" "b.el")))))
+
+(ert-deftest em-glob-test/match-recursive ()
+  "Test that \"**/\" recursively matches directories."
+  (with-fake-files '("a.el" "b.el" "ccc.el" "d.txt" "dir/a.el" "dir/sub/a.el"
+                     "dir/symlink/a.el" "symlink/a.el" "symlink/sub/a.el")
+    (should (equal (eshell-extended-glob "**/a.el")
+                   '("a.el" "dir/a.el" "dir/sub/a.el")))))
+
+(ert-deftest em-glob-test/match-recursive-follow-symlinks ()
+  "Test that \"***/\" recursively matches directories, following symlinks."
+  (with-fake-files '("a.el" "b.el" "ccc.el" "d.txt" "dir/a.el" "dir/sub/a.el"
+                     "dir/symlink/a.el" "symlink/a.el" "symlink/sub/a.el")
+    (should (equal (eshell-extended-glob "***/a.el")
+                   '("a.el" "dir/a.el" "dir/sub/a.el" "dir/symlink/a.el"
+                     "symlink/a.el" "symlink/sub/a.el")))))
+
+(ert-deftest em-glob-test/match-recursive-mixed ()
+  "Test combination of \"**/\" and \"***/\"."
+  (with-fake-files '("dir/a.el" "dir/sub/a.el" "dir/sub2/a.el"
+                     "dir/symlink/a.el" "dir/sub/symlink/a.el" "symlink/a.el"
+                     "symlink/sub/a.el" "symlink/sub/symlink/a.el")
+    (should (equal (eshell-extended-glob "**/sub/***/a.el")
+                   '("dir/sub/a.el" "dir/sub/symlink/a.el")))
+    (should (equal (eshell-extended-glob "***/sub/**/a.el")
+                   '("dir/sub/a.el" "symlink/sub/a.el")))))
+
+(ert-deftest em-glob-test/match-character-set-individual ()
+  "Test \"[...]\" for individual characters."
+  (with-fake-files '("a.el" "b.el" "c.el" "d.el" "dir/a.el")
+    (should (equal (eshell-extended-glob "[ab].el")
+                   '("a.el" "b.el")))
+    (should (equal (eshell-extended-glob "[^ab].el")
+                   '("c.el" "d.el")))))
+
+(ert-deftest em-glob-test/match-character-set-range ()
+  "Test \"[...]\" for character ranges."
+  (with-fake-files '("a.el" "b.el" "c.el" "d.el" "dir/a.el")
+    (should (equal (eshell-extended-glob "[a-c].el")
+                   '("a.el" "b.el" "c.el")))
+    (should (equal (eshell-extended-glob "[^a-c].el")
+                   '("d.el")))))
+
+(ert-deftest em-glob-test/match-character-set-class ()
+  "Test \"[...]\" for character classes."
+  (with-fake-files '("1.el" "a.el" "b.el" "c.el" "dir/a.el")
+    (should (equal (eshell-extended-glob "[[:alpha:]].el")
+                   '("a.el" "b.el" "c.el")))
+    (should (equal (eshell-extended-glob "[^[:alpha:]].el")
+                   '("1.el")))))
+
+(ert-deftest em-glob-test/match-character-set-mixed ()
+  "Test \"[...]\" with multiple kinds of members at once."
+  (with-fake-files '("1.el" "a.el" "b.el" "c.el" "d.el" "dir/a.el")
+    (should (equal (eshell-extended-glob "[ac-d[:digit:]].el")
+                   '("1.el" "a.el" "c.el" "d.el")))
+    (should (equal (eshell-extended-glob "[^ac-d[:digit:]].el")
+                   '("b.el")))))
+
+(ert-deftest em-glob-test/match-group-alternative ()
+  "Test \"(x|y)\" matches either \"x\" or \"y\"."
+  (with-fake-files '("em-alias.el" "em-banner.el" "esh-arg.el" "misc.el"
+                     "test/em-xtra.el")
+    (should (equal (eshell-extended-glob "e(m|sh)-*.el")
+                   '("em-alias.el" "em-banner.el" "esh-arg.el")))))
+
+(ert-deftest em-glob-test/match-n-or-more-characters ()
+  "Test that \"x#\" and \"x#\" match zero or more instances of \"x\"."
+  (with-fake-files '("h.el" "ha.el" "hi.el" "hii.el" "dir/hi.el")
+    (should (equal (eshell-extended-glob "hi#.el")
+                   '("h.el" "hi.el" "hii.el")))
+    (should (equal (eshell-extended-glob "hi##.el")
+                   '("hi.el" "hii.el")))))
+
+(ert-deftest em-glob-test/match-n-or-more-groups ()
+  "Test that \"(x)#\" and \"(x)#\" match zero or more instances of \"(x)\"."
+  (with-fake-files '("h.el" "ha.el" "hi.el" "hii.el" "dir/hi.el")
+    (should (equal (eshell-extended-glob "hi#.el")
+                   '("h.el" "hi.el" "hii.el")))
+    (should (equal (eshell-extended-glob "hi##.el")
+                   '("hi.el" "hii.el")))))
+
+(ert-deftest em-glob-test/match-n-or-more-character-sets ()
+  "Test that \"[x]#\" and \"[x]#\" match zero or more instances of \"[x]\"."
+  (with-fake-files '("w.el" "wh.el" "wha.el" "whi.el" "whaha.el" "dir/wha.el")
+    (should (equal (eshell-extended-glob "w[ah]#.el")
+                   '("w.el" "wh.el" "wha.el" "whaha.el")))
+    (should (equal (eshell-extended-glob "w[ah]##.el")
+                   '("wh.el" "wha.el" "whaha.el")))))
+
+(ert-deftest em-glob-test/match-x-but-not-y ()
+  "Test that \"x~y\" matches \"x\" but not \"y\"."
+  (with-fake-files '("1" "12" "123" "42" "dir/1")
+    (should (equal (eshell-extended-glob "[[:digit:]]##~4?")
+                   '("1" "12" "123")))))
+
+(ert-deftest em-glob-test/no-matches ()
+  "Test behavior when a glob fails to match any files."
+  (with-fake-files '("foo.el" "bar.el")
+    (should (equal (eshell-extended-glob "*.txt")
+                   "*.txt"))
+    (let ((eshell-error-if-no-glob t))
+      (should-error (eshell-extended-glob "*.txt")))))
+
+;; em-glob-tests.el ends here
-- 
2.25.1


[-- Attachment #3: 0002-Add-unit-tests-and-documentation-for-Eshell-predicat.patch --]
[-- Type: text/plain, Size: 38424 bytes --]

From eab9996380212a0f6ff7668d8e8a22261591656c Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sat, 19 Mar 2022 12:41:13 -0700
Subject: [PATCH 2/3] Add unit tests and documentation for Eshell
 predicates/modifiers

* lisp/eshell/esh-cmd.el (eshell-eval-argument): New function.

* lisp/eshell/em-pred.el (eshell-predicate-alist): Change socket char
to '=', since 's' conflicts with setuid.
(eshell-modifier-alist): Fix 'E' (eval) modifier by using
'eshell-eval-argument'.  Also improve performance of 'O' (reversed
sort) modifier.
(eshell-modifier-help-string): Fix documentation of global
substitution modifier.
(eshell-pred-substitute): Fix infinite loop in some global
substitutions.
(eshell-join-members): Fix joining with implicit " " delimiter.

* test/lisp/eshell/em-pred-tests.el: New file.

* doc/misc/eshell.texi (Argument Predication): New section.
---
 doc/misc/eshell.texi              | 240 ++++++++++++++
 lisp/eshell/em-pred.el            |  35 +-
 lisp/eshell/esh-cmd.el            |   8 +
 test/lisp/eshell/em-pred-tests.el | 521 ++++++++++++++++++++++++++++++
 4 files changed, 782 insertions(+), 22 deletions(-)
 create mode 100644 test/lisp/eshell/em-pred-tests.el

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 0b52c73176..2b49ddb03c 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1002,6 +1002,7 @@ Expansion
 @menu
 * Dollars Expansion::
 * Globbing::
+* Argument Predication and Modification::
 @end menu
 
 @node Dollars Expansion
@@ -1175,6 +1176,245 @@ Globbing
 
 @end table
 
+@node Argument Predication and Modification
+@section Argument Predication and Modification
+@cindex argument predication
+@cindex argument modification
+Eshell supports @dfn{argument predication}, to filter elements of a
+glob, and @dfn{argument modification}, to manipulate argument values.
+These are similar to glob qualifiers in Zsh (@pxref{Glob Qualifiers, ,
+, zsh, The Z Shell Manual}).
+
+Predicates and modifiers are introduced with @code{(@var{filters})}
+after any list argument, where @var{filters} is a list of predicates
+or modifiers.  For example, @samp{*(.)} expands to all regular files
+in the current directory and @samp{*(^@@:U^u0)} expands to all
+non-symlinks not owned by @code{root}, upper-cased.
+
+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}).
+
+@menu
+* Argument Predicates::
+* Argument Modifiers::
+@end menu
+
+@node Argument Predicates
+@subsection Argument Predicates
+You can use argument predicates to filter lists of file names based on
+various properties of those files.  This is most useful when combined
+with globbing, but can be used on any list of files names.  Eshell
+supports the following argument predicates:
+
+@table @asis
+
+@item @code{/}
+Matches directories.
+
+@item @code{.} @r{(Period)}
+Matches regular files.
+
+@item @code{@@}
+Matches symbolic links.
+
+@item @code{=}
+Matches sockets.
+
+@item @code{p}
+Matches named pipes.
+
+@item @code{%}
+Matches block or character devices.
+
+@item @code{%b}
+Matches block devices.
+
+@item @code{%c}
+Matches character devices.
+
+@item @code{*}
+Matches regular files that can be executed by the current user.
+
+@item @code{r}
+@item @code{A}
+@item @code{R}
+Matches files that are readable by their owners (@code{r}), their
+groups (@code{A}), or the world (@code{R}).
+
+@item @code{w}
+@item @code{I}
+@item @code{W}
+Matches files that are writable by their owners (@code{w}), their
+groups (@code{I}), or the world (@code{W}).
+
+@item @code{x}
+@item @code{E}
+@item @code{X}
+Matches files that are executable by their owners (@code{x}), their
+groups (@code{E}), or the world (@code{X}).
+
+@item @code{s}
+Matches files with the setuid flag set.
+
+@item @code{S}
+Matches files with the setgid flag set.
+
+@item @code{t}
+Matches files with the sticky bit set.
+
+@item @code{U}
+Matches files owned by the current effective user ID.
+
+@item @code{l@option{[+-]}@var{n}}
+Matches files with @var{n} links.  With @option{+} (or @option{-}),
+matches files with more than (or less than) @var{n} links,
+respectively.
+
+@item @code{u@var{uid}}
+@item @code{u'@var{user-name}'}
+Matches files owned by user ID @var{uid} or user name @var{user-name}.
+
+@item @code{g@var{gid}}
+@item @code{g'@var{group-name}'}
+Matches files owned by group ID @var{gid} or group name
+@var{group-name}.
+
+@item @code{a@option{[@var{unit}]}@option{[+-]}@var{n}}
+@item @code{a@option{[+-]}'@var{file}'}
+Matches files last accessed exactly @var{n} days ago.  With @option{+}
+(or @option{-}), matches files accessed more than (or less than)
+@var{n} days ago, respectively.
+
+With @var{unit}, @var{n} is a quantity in that unit of time, so
+@samp{aw-1} matches files last accessed within one week.  @var{unit}
+can be @code{M} (30-day months), @code{w} (weeks), @code{h} (hours),
+@code{m} (minutes), or @code{s} (seconds).
+
+If @var{file} is specified instead, compare against the modification
+time of @file{file}.  Thus, @samp{a-'hello.txt'} matches all files
+accessed after @file{hello.txt} was last accessed.
+
+@item @code{m@option{[@var{unit}]}@option{[+-]}@var{n}}
+@item @code{m@option{[+-]}'@var{file}'}
+Like @code{a}, but examines modification time.
+
+@item @code{c@option{[@var{unit}]}@option{[+-]}@var{n}}
+@item @code{c@option{[+-]}'@var{file}'}
+Like @code{a}, but examines status change time.
+
+@item @code{L@option{[@var{unit}]}@option{[+-]}@var{n}}
+Matches files exactly @var{n} bytes in size.  With @option{+} (or
+@option{-}), matches files larger than (or smaller than) @var{n}
+bytes, respectively.
+
+With @var{unit}, @var{n} is a quantity in that unit of size, so
+@samp{Lm+5} matches files larger than 5 MiB in size.  @var{unit} can
+be one of the following (case-insensitive) characters: @code{m}
+(megabytes), @code{k} (kilobytes), or @code{p} (512-byte blocks).
+
+@end table
+
+The @code{^} and @code{-} operators are not argument predicates
+themselves, but they modify the behavior of all subsequent predicates.
+@code{^} inverts the meaning of subsequent predicates, so
+@samp{*(^RWX)} expands to all files whose permissions disallow the
+world from accessing them in any way (i.e., reading, writing to, or
+modifying them).  When examining a symbolic link, @code{-} applies the
+subsequent predicates to the link's target instead of the link itself.
+
+@node Argument Modifiers
+@subsection Argument Modifiers
+You can use argument modifiers to manipulate argument values.  For
+example, you can sort lists, remove duplicate values, capitalize
+words, etc.  All argument modifiers are prefixed by @code{:}, so
+@samp{$exec-path(:h:u:x/^\/home/)} lists all of the unique parent
+directories of the elements in @code{exec-path}, excluding those in
+@file{/home}.
+
+@table @code
+
+@item E
+Re-evaluates the value as an Eshell argument.  For example, if
+@var{foo} is @code{"$@{echo hi@}"}, then the result of @samp{$foo(:E)}
+is @code{hi}.
+
+@item L
+Converts the value to lower case.
+
+@item U
+Converts the value to upper case.
+
+@item C
+Capitalizes the value.
+
+@item h
+Treating the value as a file name, gets the directory name (the
+``head'').  For example, @samp{foo/bar/baz.el(:h)} expands to
+@code{foo/bar/}.
+
+@item t
+Treating the value as a file name, gets the base name (the ``tail'').
+For example, @samp{foo/bar/baz.el(:h)} expands to @code{baz.el}.
+
+@item e
+Treating the value as a file name, gets the final extension of the
+file, excluding the dot.  For example, @samp{foo.tar.gz(:e)}
+expands to @code{gz}.
+
+@item r
+Treating the value as a file name, gets the file name excluding the
+final extension.  For example, @samp{foo/bar/baz.tar.gz(:r)} expands
+to @code{foo/bar/baz.tar}.
+
+@item q
+Marks that the value should be interpreted by Eshell literally, so
+that any special characters like @code{$} no longer have any special
+meaning.
+
+@item s/@var{pattern}/@var{replace}/
+Replaces the first instance of the regular expression @var{pattern}
+with @var{replace}.  Signals an error if no match is found.
+
+@item gs/@var{pattern}/@var{replace}/
+Replaces all instances of the regular expression @var{pattern} with
+@var{replace}.
+
+@item i/@var{pattern}/
+Filters a list of values to include only the elements matching the
+regular expression @var{pattern}.
+
+@item x/@var{pattern}/
+Filters a list of values to exclude all the elements matching the
+regular expression @var{pattern}.
+
+@item S
+@item S/@var{pattern}/
+Splits the value using the regular expression @var{pattern} as a
+delimiter.  If @var{pattern} is omitted, split on spaces.
+
+@item j
+@item j/@var{delim}/
+Joins a list of values, inserting the string @var{delim} between each
+value.  If @var{delim} is omitted, use a single space as the
+delimiter.
+
+@item o
+Sorts a list of strings in ascending lexicographic order, comparing
+pairs of characters according to their character codes (@pxref{Text
+Comparison, , , elisp, The Emacs Lisp Reference Manual}).
+
+@item O
+Sorts a list of strings in descending lexicographic order.
+
+@item u
+Removes any duplicate elements from a list of values.
+
+@item R
+Reverses the order of a list of values.
+
+@end table
+
 @node Input/Output
 @chapter Input/Output
 Since Eshell does not communicate with a terminal like most command
diff --git a/lisp/eshell/em-pred.el b/lisp/eshell/em-pred.el
index 970329e12a..8afc86dd41 100644
--- a/lisp/eshell/em-pred.el
+++ b/lisp/eshell/em-pred.el
@@ -68,7 +68,7 @@ eshell-pred-load-hook
 (defcustom eshell-predicate-alist
   '((?/ . (eshell-pred-file-type ?d))   ; directories
     (?. . (eshell-pred-file-type ?-))   ; regular files
-    (?s . (eshell-pred-file-type ?s))   ; sockets
+    (?= . (eshell-pred-file-type ?s))   ; sockets
     (?p . (eshell-pred-file-type ?p))   ; named pipes
     (?@ . (eshell-pred-file-type ?l))   ; symbolic links
     (?% . (eshell-pred-file-type ?%))   ; allow user to specify (c def.)
@@ -97,8 +97,8 @@ eshell-predicate-alist
                  (not (file-symlink-p file))
                  (file-executable-p file))))
     (?l . (eshell-pred-file-links))
-    (?u . (eshell-pred-user-or-group ?u "user" 2 'eshell-user-id))
-    (?g . (eshell-pred-user-or-group ?g "group" 3 'eshell-group-id))
+    (?u . (eshell-pred-user-or-group ?u "user" 2 #'eshell-user-id))
+    (?g . (eshell-pred-user-or-group ?g "group" 3 #'eshell-group-id))
     (?a . (eshell-pred-file-time ?a "access" 4))
     (?m . (eshell-pred-file-time ?m "modification" 5))
     (?c . (eshell-pred-file-time ?c "change" 6))
@@ -111,12 +111,7 @@ eshell-predicate-alist
   :risky t)
 
 (defcustom eshell-modifier-alist
-  '((?E . (lambda (lst)
-            (mapcar
-             (lambda (str)
-               (eshell-stringify
-                (car (eshell-parse-argument str))))
-             lst)))
+  '((?E . (lambda (lst) (mapcar #'eshell-eval-argument lst)))
     (?L . (lambda (lst) (mapcar #'downcase lst)))
     (?U . (lambda (lst) (mapcar #'upcase lst)))
     (?C . (lambda (lst) (mapcar #'capitalize lst)))
@@ -129,10 +124,10 @@ eshell-modifier-alist
     (?q . (lambda (lst) (mapcar #'eshell-escape-arg lst)))
     (?u . (lambda (lst) (seq-uniq lst)))
     (?o . (lambda (lst) (sort lst #'string-lessp)))
-    (?O . (lambda (lst) (nreverse (sort lst #'string-lessp))))
+    (?O . (lambda (lst) (sort lst #'string-greaterp)))
     (?j . (eshell-join-members))
     (?S . (eshell-split-members))
-    (?R . 'reverse)
+    (?R . #'reverse)
     (?g . (progn
 	    (forward-char)
 	    (if (eq (char-before) ?s)
@@ -142,7 +137,7 @@ eshell-modifier-alist
   "A list of modifiers than can be applied to an argument expansion.
 The format of each entry is
 
-  (CHAR ENTRYWISE-P MODIFIER-FUNC-SEXP)"
+  (CHAR . MODIFIER-FUNC-SEXP)"
   :type '(repeat (cons character sexp))
   :risky t)
 
@@ -217,8 +212,8 @@ eshell-modifier-help-string
   i/PAT/  exclude all members not matching PAT
   x/PAT/  exclude all members matching PAT
 
-  s/pat/match/  substitute PAT with MATCH
-  g/pat/match/  substitute PAT with MATCH for all occurrences
+  s/pat/match/   substitute PAT with MATCH
+  gs/pat/match/  substitute PAT with MATCH for all occurrences
 
 EXAMPLES:
   *.c(:o)  sorted list of .c files")
@@ -534,18 +529,14 @@ eshell-pred-substitute
 	(lambda (lst)
 	  (mapcar
            (lambda (str)
-             (let ((i 0))
-               (while (setq i (string-match match str i))
-                 (setq str (replace-match replace t nil str))))
-             str)
+             (replace-regexp-in-string match replace str t))
            lst))
       (lambda (lst)
 	(mapcar
          (lambda (str)
            (if (string-match match str)
-               (setq str (replace-match replace t nil str))
-             (error (concat str ": substitution failed")))
-           str)
+               (replace-match replace t nil str)
+             (error (concat str ": substitution failed"))))
          lst)))))
 
 (defun eshell-include-members (&optional invert-p)
@@ -568,7 +559,7 @@ eshell-join-members
   (let ((delim (char-after))
 	str end)
     (if (not (memq delim '(?' ?/)))
-	(setq delim " ")
+	(setq str " ")
       (forward-char)
       (setq end (eshell-find-delimiter delim delim nil nil t)
 	    str (buffer-substring-no-properties (point) end))
diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index 8be1136e31..42616e7037 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -1002,6 +1002,14 @@ eshell-invoke-directly
   (let ((base (cadr (nth 2 (nth 2 (cadr command))))))
     (eshell--invoke-command-directly base)))
 
+(defun eshell-eval-argument (argument)
+  "Evaluate a single Eshell ARGUMENT and return the result."
+  (let* ((form (eshell-with-temp-command argument
+                 (eshell-parse-argument)))
+         (result (eshell-do-eval form t)))
+    (cl-assert (eq (car result) 'quote))
+    (cadr result)))
+
 (defun eshell-eval-command (command &optional input)
   "Evaluate the given COMMAND iteratively."
   (if eshell-current-command
diff --git a/test/lisp/eshell/em-pred-tests.el b/test/lisp/eshell/em-pred-tests.el
new file mode 100644
index 0000000000..74dad9f8b8
--- /dev/null
+++ b/test/lisp/eshell/em-pred-tests.el
@@ -0,0 +1,521 @@
+;;; em-pred-tests.el --- em-pred test suite  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2022 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; Tests for Eshell's argument predicates/modifiers.
+
+;;; Code:
+
+(require 'ert)
+(require 'esh-mode)
+(require 'eshell)
+
+(require 'eshell-tests-helpers
+         (expand-file-name "eshell-tests-helpers"
+                           (file-name-directory (or load-file-name
+                                                    default-directory))))
+
+(defvar eshell-test-value nil)
+
+(defun eshell-eval-predicate (initial-value predicate)
+  "Evaluate PREDICATE on INITIAL-VALUE, returning the result.
+PREDICATE is an Eshell argument predicate/modifier."
+  (let ((eshell-test-value initial-value))
+    (with-temp-eshell
+     (eshell-insert-command
+      (format "setq eshell-test-value $eshell-test-value(%s)" predicate)))
+    eshell-test-value))
+
+(defun eshell-parse-file-name-attributes (file)
+  "Parse a fake FILE name to determine its attributes.
+Fake file names are file names beginning with \"/fake/\".  This
+allows defining file names for fake files with various properties
+to query via predicates.  Attributes are written as a
+comma-separate list of ATTR=VALUE pairs as the file's base name,
+like:
+
+  /fake/type=-,modes=0755.el
+
+The following attributes are recognized:
+
+  * \"type\": A single character describing the file type;
+    accepts the same values as the first character of the file
+    modes in `ls -l'.
+  * \"modes\": The file's permission modes, in octal.
+  * \"links\": The number of links to this file.
+  * \"uid\": The UID of the file's owner.
+  * \"gid\": The UID of the file's group.
+  * \"atime\": The time the file was last accessed, in seconds
+    since the UNIX epoch.
+  * \"mtime\": As \"atime\", but for modification time.
+  * \"ctime\": As \"atime\", but for inode change time.
+  * \"size\": The file's size in bytes."
+  (mapcar (lambda (i)
+            (pcase (split-string i "=")
+              (`("modes" ,modes)
+               (cons 'modes (string-to-number modes 8)))
+              (`(,(and (or "links" "uid" "gid" "size") key) ,value)
+               (cons (intern key) (string-to-number value)))
+              (`(,(and (or "atime" "mtime" "ctime") key) ,value)
+               (cons (intern key) (time-convert (string-to-number value))))
+              (`(,key ,value)
+               (cons (intern key) value))
+              (_ (error "invalid format %S" i))))
+          (split-string (file-name-base file) ",")))
+
+(defmacro eshell-partial-let-func (overrides &rest body)
+  "Temporarily bind to FUNCTION-NAMEs and evaluate BODY.
+This is roughly analogous to advising functions, but only does so
+while BODY is executing, and only calls NEW-FUNCTION if its first
+argument is a string beginning with \"/fake/\".
+
+This allows selectively overriding functions to test file
+properties with fake files without altering the functions'
+behavior for real files.
+
+\(fn ((FUNCTION-NAME NEW-FUNCTION) ...) BODY...)"
+  (declare (indent 1))
+  `(cl-letf
+       ,(mapcar
+         (lambda (override)
+           (let ((orig-function (symbol-function (car override))))
+             `((symbol-function #',(car override))
+               (lambda (file &rest rest)
+                 (apply
+                  (if (and (stringp file) (string-prefix-p "/fake/" file))
+                      ,(cadr override)
+                    ,orig-function)
+                  file rest)))))
+         overrides)
+     ,@body))
+
+(defmacro eshell-with-file-attributes-from-name (&rest body)
+  "Temporarily override file attribute functions and evaluate BODY."
+  (declare (indent 0))
+  `(eshell-partial-let-func
+       ((file-attributes
+         (lambda (file &optional _id-format)
+           (let ((attrs (eshell-parse-file-name-attributes file)))
+             (list (equal (alist-get 'type attrs) "d")
+                   (or (alist-get 'links attrs) 1)
+                   (or (alist-get 'uid attrs) 0)
+                   (or (alist-get 'gid attrs) 0)
+                   (or (alist-get 'atime attrs) nil)
+                   (or (alist-get 'mtime attrs) nil)
+                   (or (alist-get 'ctime attrs) nil)
+                   (or (alist-get 'size attrs) 0)
+                   (format "%s---------" (or (alist-get 'type attrs) "-"))
+                   nil 0 0))))
+        (file-modes
+         (lambda (file _nofollow)
+           (let ((attrs (eshell-parse-file-name-attributes file)))
+             (or (alist-get 'modes attrs) 0))))
+        (file-exists-p #'always)
+        (file-regular-p
+         (lambda (file)
+           (let ((attrs (eshell-parse-file-name-attributes file)))
+             (member (or (alist-get 'type attrs) "-") '("-" "l")))))
+        (file-symlink-p
+         (lambda (file)
+           (let ((attrs (eshell-parse-file-name-attributes file)))
+             (equal (alist-get 'type attrs) "l"))))
+        (file-executable-p
+         (lambda (file)
+           (let ((attrs (eshell-parse-file-name-attributes file)))
+             ;; For simplicity, just return whether the file is
+             ;; world-executable.
+             (= (logand (or (alist-get 'modes attrs) 0) 1) 1)))))
+     ,@body))
+
+;;; Tests:
+
+\f
+;; Argument predicates
+
+(ert-deftest em-pred-test/predicate-file-types ()
+  "Test file type predicates."
+  (eshell-with-file-attributes-from-name
+    (let ((files (mapcar (lambda (i) (format "/fake/type=%s" i))
+                         '("b" "c" "d/" "p" "s" "l" "-"))))
+      (should (equal (eshell-eval-predicate files "%")
+                     '("/fake/type=b" "/fake/type=c")))
+      (should (equal (eshell-eval-predicate files "%b") '("/fake/type=b")))
+      (should (equal (eshell-eval-predicate files "%c") '("/fake/type=c")))
+      (should (equal (eshell-eval-predicate files "/")  '("/fake/type=d/")))
+      (should (equal (eshell-eval-predicate files ".")  '("/fake/type=-")))
+      (should (equal (eshell-eval-predicate files "p")  '("/fake/type=p")))
+      (should (equal (eshell-eval-predicate files "=")  '("/fake/type=s")))
+      (should (equal (eshell-eval-predicate files "@")  '("/fake/type=l"))))))
+
+(ert-deftest em-pred-test/predicate-executable ()
+  "Test that \"*\" matches only regular, non-symlink executable files."
+  (eshell-with-file-attributes-from-name
+    (let ((files '("/fake/modes=0777" "/fake/modes=0666"
+                   "/fake/type=d,modes=0777" "/fake/type=l,modes=0777")))
+      (should (equal (eshell-eval-predicate files "*")
+                     '("/fake/modes=0777"))))))
+
+(defmacro em-pred-test--file-modes-deftest (name mode-template predicates
+                                                 &optional docstring)
+  "Define NAME as a file-mode test.
+MODE-TEMPLATE is a format string to convert an integer from 0 to
+7 to an octal file mode.  PREDICATES is a list of strings for the
+read, write, and execute predicates to query the file's modes."
+  (declare (indent 4) (doc-string 4))
+  `(ert-deftest ,name ()
+     ,docstring
+     (eshell-with-file-attributes-from-name
+       (let ((file-template (concat "/fake/modes=" ,mode-template)))
+         (cl-flet ((make-files (perms)
+                               (mapcar (lambda (i) (format file-template i))
+                                       perms)))
+           (pcase-let ((files (make-files (number-sequence 0 7)))
+                       (`(,read ,write ,exec) ,predicates))
+             (should (equal (eshell-eval-predicate files read)
+                            (make-files '(4 5 6 7))))
+             (should (equal (eshell-eval-predicate files (concat "^" read))
+                            (make-files '(0 1 2 3))))
+             (should (equal (eshell-eval-predicate files write)
+                            (make-files '(2 3 6 7))))
+             (should (equal (eshell-eval-predicate files (concat "^" write))
+                            (make-files '(0 1 4 5))))
+             (should (equal (eshell-eval-predicate files exec)
+                            (make-files '(1 3 5 7))))
+             (should (equal (eshell-eval-predicate files (concat "^" exec))
+                            (make-files '(0 2 4 6))))))))))
+
+(em-pred-test--file-modes-deftest em-pred-test/predicate-file-modes-owner
+    "0%o00" '("r" "w" "x")
+    "Test predicates for file permissions for the owner.")
+
+(em-pred-test--file-modes-deftest em-pred-test/predicate-file-modes-group
+    "00%o0" '("A" "I" "E")
+    "Test predicates for file permissions for the group.")
+
+(em-pred-test--file-modes-deftest em-pred-test/predicate-file-modes-world
+    "000%o" '("R" "W" "X")
+    "Test predicates for file permissions for the world.")
+
+(em-pred-test--file-modes-deftest em-pred-test/predicate-file-modes-flags
+    "%o000" '("s" "S" "t")
+    "Test predicates for \"s\" (setuid), \"S\" (setgid), and \"t\" (sticky).")
+
+(ert-deftest em-pred-test/predicate-effective-uid ()
+  "Test that \"U\" matches files owned by the effective UID."
+  (eshell-with-file-attributes-from-name
+    (cl-letf (((symbol-function 'user-uid) (lambda () 1)))
+      (let ((files '("/fake/uid=1" "/fake/uid=2")))
+        (should (equal (eshell-eval-predicate files "U")
+                       '("/fake/uid=1")))))))
+
+(ert-deftest em-pred-test/predicate-links ()
+  "Test that \"l\" filters by number of links."
+  (eshell-with-file-attributes-from-name
+    (let ((files '("/fake/links=1" "/fake/links=2" "/fake/links=3")))
+      (should (equal (eshell-eval-predicate files "l1")
+                     '("/fake/links=1")))
+      (should (equal (eshell-eval-predicate files "l+1")
+                     '("/fake/links=2" "/fake/links=3")))
+      (should (equal (eshell-eval-predicate files "l-3")
+                     '("/fake/links=1" "/fake/links=2"))))))
+
+(ert-deftest em-pred-test/predicate-uid ()
+  "Test that \"u\" filters by UID/user name."
+  (eshell-with-file-attributes-from-name
+    (let ((files '("/fake/uid=1" "/fake/uid=2"))
+          (user-names '("root" "one" "two")))
+      (should (equal (eshell-eval-predicate files "u1")
+                     '("/fake/uid=1")))
+      (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 ()
+  "Test that \"g\" filters by GID/group name."
+  (eshell-with-file-attributes-from-name
+    (let ((files '("/fake/gid=1" "/fake/gid=2"))
+          (group-names '("root" "one" "two")))
+      (should (equal (eshell-eval-predicate files "g1")
+                     '("/fake/gid=1")))
+      (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
+                                           &optional docstring)
+  "Define NAME as a file-time test.
+FILE-ATTRIBUTE is the file's attribute to set (e.g. \"atime\").
+PREDICATE is the predicate used to query that attribute."
+  (declare (indent 4) (doc-string 4))
+  `(ert-deftest ,name ()
+     ,docstring
+     (eshell-with-file-attributes-from-name
+       (cl-flet ((make-file (time)
+                            (format "/fake/%s=%d" ,file-attribute time)))
+         (let* ((now (time-convert nil 'integer))
+                (yesterday (- now 86400))
+                (files (mapcar #'make-file (list now yesterday))))
+           ;; Test comparison against a number of days.
+           (should (equal (eshell-eval-predicate
+                           files (concat ,predicate "-1"))
+                          (mapcar #'make-file (list now))))
+           (should (equal (eshell-eval-predicate
+                           files (concat ,predicate "+1"))
+                          (mapcar #'make-file (list yesterday))))
+           (should (equal (eshell-eval-predicate
+                           files (concat ,predicate "+2"))
+                          nil))
+           ;; Test comparison against a number of hours.
+           (should (equal (eshell-eval-predicate
+                           files (concat ,predicate "h-1"))
+                          (mapcar #'make-file (list now))))
+           (should (equal (eshell-eval-predicate
+                           files (concat ,predicate "h+1"))
+                          (mapcar #'make-file (list yesterday))))
+           (should (equal (eshell-eval-predicate
+                           files (concat ,predicate "+48"))
+                          nil))
+           ;; Test comparison against another file.
+           (should (equal (eshell-eval-predicate
+                           files (format "%s-'%s'" ,predicate (make-file now)))
+                          nil))
+           (should (equal (eshell-eval-predicate
+                           files (format "%s+'%s'" ,predicate (make-file now)))
+                          (mapcar #'make-file (list yesterday)))))))))
+
+(em-pred-test--time-deftest em-pred-test/predicate-access-time
+    "atime" "a"
+    "Test that \"a\" filters by access time.")
+
+(em-pred-test--time-deftest em-pred-test/predicate-modification-time
+    "mtime" "m"
+    "Test that \"m\" filters by change time.")
+
+(em-pred-test--time-deftest em-pred-test/predicate-change-time
+    "ctime" "c"
+    "Test that \"c\" filters by change time.")
+
+(ert-deftest em-pred-test/predicate-size ()
+  "Test that \"L\" filters by file size."
+  (eshell-with-file-attributes-from-name
+    (let ((files '("/fake/size=0"
+                   ;; 1 and 2 KiB.
+                   "/fake/size=1024" "/fake/size=2048"
+                   ;; 1 and 2 MiB.
+                   "/fake/size=1048576" "/fake/size=2097152")))
+      ;; Size in bytes.
+      (should (equal (eshell-eval-predicate files "L2048")
+                     '("/fake/size=2048")))
+      (should (equal (eshell-eval-predicate files "L+2048")
+                     '("/fake/size=1048576" "/fake/size=2097152")))
+      (should (equal (eshell-eval-predicate files "L-2048")
+                     '("/fake/size=0" "/fake/size=1024")))
+      ;; Size in blocks.
+      (should (equal (eshell-eval-predicate files "Lp4")
+                     '("/fake/size=2048")))
+      (should (equal (eshell-eval-predicate files "Lp+4")
+                     '("/fake/size=1048576" "/fake/size=2097152")))
+      (should (equal (eshell-eval-predicate files "Lp-4")
+                     '("/fake/size=0" "/fake/size=1024")))
+      ;; Size in KiB.
+      (should (equal (eshell-eval-predicate files "Lk2")
+                     '("/fake/size=2048")))
+      (should (equal (eshell-eval-predicate files "Lk+2")
+                     '("/fake/size=1048576" "/fake/size=2097152")))
+      (should (equal (eshell-eval-predicate files "Lk-2")
+                     '("/fake/size=0" "/fake/size=1024")))
+      ;; Size in MiB.
+      (should (equal (eshell-eval-predicate files "LM1")
+                     '("/fake/size=1048576")))
+      (should (equal (eshell-eval-predicate files "LM+1")
+                     '("/fake/size=2097152")))
+      (should (equal (eshell-eval-predicate files "LM-1")
+                     '("/fake/size=0" "/fake/size=1024" "/fake/size=2048"))))))
+
+\f
+;; Argument modifiers
+
+(ert-deftest em-pred-test/modifier-eval ()
+  "Test that \":E\" re-evaluates the value."
+  (should (equal (eshell-eval-predicate "${echo hi}" ":E") "hi"))
+  (should (equal (eshell-eval-predicate
+                  '("${echo hi}" "$(upcase \"bye\")") ":E")
+                 '("hi" "BYE"))))
+
+(ert-deftest em-pred-test/modifier-downcase ()
+  "Test that \":L\" downcases values."
+  (should (equal (eshell-eval-predicate "FOO" ":L") "foo"))
+  (should (equal (eshell-eval-predicate '("FOO" "BAR") ":L")
+                 '("foo" "bar"))))
+
+(ert-deftest em-pred-test/modifier-upcase ()
+  "Test that \":U\" upcases values."
+  (should (equal (eshell-eval-predicate "foo" ":U") "FOO"))
+  (should (equal (eshell-eval-predicate '("foo" "bar") ":U")
+                 '("FOO" "BAR"))))
+
+(ert-deftest em-pred-test/modifier-capitalize ()
+  "Test that \":C\" capitalizes values."
+  (should (equal (eshell-eval-predicate "foo bar" ":C") "Foo Bar"))
+  (should (equal (eshell-eval-predicate '("foo bar" "baz") ":C")
+                 '("Foo Bar" "Baz"))))
+
+(ert-deftest em-pred-test/modifier-dirname ()
+  "Test that \":h\" returns the dirname."
+  (should (equal (eshell-eval-predicate "/path/to/file.el" ":h") "/path/to/"))
+  (should (equal (eshell-eval-predicate
+                  '("/path/to/file.el" "/other/path/") ":h")
+                 '("/path/to/" "/other/path/"))))
+
+(ert-deftest em-pred-test/modifier-basename ()
+  "Test that \":t\" returns the basename."
+  (should (equal (eshell-eval-predicate "/path/to/file.el" ":t") "file.el"))
+  (should (equal (eshell-eval-predicate
+                  '("/path/to/file.el" "/other/path/") ":t")
+                 '("file.el" ""))))
+
+(ert-deftest em-pred-test/modifier-extension ()
+  "Test that \":e\" returns the extension."
+  (should (equal (eshell-eval-predicate "/path/to/file.el" ":e") "el"))
+  (should (equal (eshell-eval-predicate
+                  '("/path/to/file.el" "/other/path/") ":e")
+                 '("el" nil))))
+
+(ert-deftest em-pred-test/modifier-sans-extension ()
+  "Test that \":r\" returns the file name san extension."
+  (should (equal (eshell-eval-predicate "/path/to/file.el" ":r")
+                 "/path/to/file"))
+  (should (equal (eshell-eval-predicate
+                  '("/path/to/file.el" "/other/path/") ":r")
+                 '("/path/to/file" "/other/path/"))))
+
+(ert-deftest em-pred-test/modifier-quote ()
+  "Test that \":q\" quotes arguments."
+  (should (equal-including-properties
+           (eshell-eval-predicate '("foo" "bar") ":q")
+           (list (eshell-escape-arg "foo") (eshell-escape-arg "bar")))))
+
+(ert-deftest 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 '("foo" "bar" "baz") ":s/[ao]/*/")
+                 '("f*o" "b*r" "b*z")))
+  (should (equal (eshell-eval-predicate '("foo" "bar" "baz") ":s|[ao]|*|")
+                 '("f*o" "b*r" "b*z"))))
+
+(ert-deftest em-pred-test/modifier-global-substitute ()
+  "Test that \":s/PAT/REP/\" replaces PAT with REP for all occurrences."
+  (should (equal (eshell-eval-predicate "foo" ":gs/a/*/") "foo"))
+  (should (equal (eshell-eval-predicate "foo" ":gs|a|*|") "foo"))
+  (should (equal (eshell-eval-predicate "bar" ":gs/a/*/") "b*r"))
+  (should (equal (eshell-eval-predicate "bar" ":gs|a|*|") "b*r"))
+  (should (equal (eshell-eval-predicate "foo" ":gs/o/O/") "fOO"))
+  (should (equal (eshell-eval-predicate '("foo" "bar" "baz") ":gs/[aeiou]/*/")
+                 '("f**" "b*r" "b*z")))
+  (should (equal (eshell-eval-predicate '("foo" "bar" "baz") ":gs|[aeiou]|*|")
+                 '("f**" "b*r" "b*z"))))
+
+(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 ()
+  "Test that \":S\" and \":S/PAT/\" split elements by spaces (or PAT)."
+  (should (equal (eshell-eval-predicate "foo bar baz" ":S")
+                 '("foo" "bar" "baz")))
+  (should (equal (eshell-eval-predicate '("foo bar" "baz") ":S")
+                 '(("foo" "bar") ("baz"))))
+  (should (equal (eshell-eval-predicate "foo-bar-baz" ":S/-/")
+                 '("foo" "bar" "baz")))
+  (should (equal (eshell-eval-predicate '("foo-bar" "baz") ":S/-/")
+                 '(("foo" "bar") ("baz")))))
+
+(ert-deftest em-pred-test/modifier-join ()
+  "Test that \":j\" and \":j/DELIM/\" join elements by spaces (or DELIM)."
+  (should (equal (eshell-eval-predicate "foo" ":j") "foo"))
+  (should (equal (eshell-eval-predicate '("foo" "bar" "baz") ":j")
+                 "foo bar baz"))
+  (should (equal (eshell-eval-predicate "foo" ":j/-/") "foo"))
+  (should (equal (eshell-eval-predicate '("foo" "bar" "baz") ":j/-/")
+                 "foo-bar-baz")))
+
+(ert-deftest em-pred-test/modifier-sort ()
+  "Test that \":o\" sorts elements in lexicographic order."
+  (should (equal (eshell-eval-predicate "foo" ":o") "foo"))
+  (should (equal (eshell-eval-predicate '("foo" "bar" "baz") ":o")
+                 '("bar" "baz" "foo"))))
+
+(ert-deftest em-pred-test/modifier-sort-reverse ()
+  "Test that \":o\" sorts elements in reverse lexicographic order."
+  (should (equal (eshell-eval-predicate "foo" ":O") "foo"))
+  (should (equal (eshell-eval-predicate '("foo" "bar" "baz") ":O")
+                 '("foo" "baz" "bar"))))
+
+(ert-deftest em-pred-test/modifier-unique ()
+  "Test that \":u\" filters out duplicate elements."
+  (should (equal (eshell-eval-predicate "foo" ":u") "foo"))
+  (should (equal (eshell-eval-predicate '("foo" "bar" "baz") ":u")
+                 '("foo" "bar" "baz")))
+  (should (equal (eshell-eval-predicate '("foo" "bar" "baz" "foo") ":u")
+                 '("foo" "bar" "baz"))))
+
+(ert-deftest em-pred-test/modifier-reverse ()
+  "Test that \":r\" reverses the order of elements."
+  (should (equal (eshell-eval-predicate "foo" ":R") "foo"))
+  (should (equal (eshell-eval-predicate '("foo" "bar" "baz") ":R")
+                 '("baz" "bar" "foo"))))
+
+\f
+;; Combinations
+
+(ert-deftest em-pred-test/combine-predicate-and-modifier ()
+  "Test combination of predicates and modifiers."
+  (eshell-with-file-attributes-from-name
+    (let ((files '("/fake/type=-.el" "/fake/type=-.txt" "/fake/type=s.el"
+                   "/fake/subdir/type=-.el")))
+      (should (equal (eshell-eval-predicate files ".:e:u")
+                     '("el" "txt"))))))
+
+;; em-pred-tests.el ends here
-- 
2.25.1


[-- Attachment #4: 0003-Add-G-argument-predicate-in-Eshell.patch --]
[-- Type: text/plain, Size: 3186 bytes --]

From 005959bbe9998185bf9600387aa308ffabba0070 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Sat, 19 Mar 2022 17:52:55 -0700
Subject: [PATCH 3/3] Add 'G' argument predicate in Eshell

* lisp/eshell/em-pred.el (eshell-predicate-alist): Add 'G' predicate.
(eshell-predicate-help-string): Document it.

* test/lisp/eshell/em-pred-tests.el
(em-pred-test/predicate-effective-gid): New test.

* doc/misc/eshell.text (Argument Predication): Document 'G' predicate.
---
 doc/misc/eshell.texi              | 3 +++
 lisp/eshell/em-pred.el            | 9 +++++----
 test/lisp/eshell/em-pred-tests.el | 8 ++++++++
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 2b49ddb03c..89f5b953f5 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1266,6 +1266,9 @@ Argument Predicates
 @item @code{U}
 Matches files owned by the current effective user ID.
 
+@item @code{G}
+Matches files owned by the current effective group ID.
+
 @item @code{l@option{[+-]}@var{n}}
 Matches files with @var{n} links.  With @option{+} (or @option{-}),
 matches files with more than (or less than) @var{n} links,
diff --git a/lisp/eshell/em-pred.el b/lisp/eshell/em-pred.el
index 8afc86dd41..eb5109b82d 100644
--- a/lisp/eshell/em-pred.el
+++ b/lisp/eshell/em-pred.el
@@ -88,10 +88,10 @@ eshell-predicate-alist
             (if (file-exists-p file)
                 (= (file-attribute-user-id (file-attributes file))
                    (user-uid)))))
-    ;; (?G . (lambda (file)               ; owned by effective gid
-    ;;         (if (file-exists-p file)
-    ;;             (= (file-attribute-user-id (file-attributes file))
-    ;;                (user-uid)))))
+    (?G . (lambda (file)               ; owned by effective gid
+            (if (file-exists-p file)
+                (= (file-attribute-group-id (file-attributes file))
+                   (group-gid)))))
     (?* . (lambda (file)
             (and (file-regular-p file)
                  (not (file-symlink-p file))
@@ -161,6 +161,7 @@ eshell-predicate-help-string
 
 OWNERSHIP:
   U               owned by effective uid
+  G               owned by effective gid
   u(UID|\\='user\\=')   owned by UID/user
   g(GID|\\='group\\=')  owned by GID/group
 
diff --git a/test/lisp/eshell/em-pred-tests.el b/test/lisp/eshell/em-pred-tests.el
index 74dad9f8b8..fbf8945215 100644
--- a/test/lisp/eshell/em-pred-tests.el
+++ b/test/lisp/eshell/em-pred-tests.el
@@ -225,6 +225,14 @@ em-pred-test/predicate-effective-uid
         (should (equal (eshell-eval-predicate files "U")
                        '("/fake/uid=1")))))))
 
+(ert-deftest em-pred-test/predicate-effective-gid ()
+  "Test that \"G\" matches files owned by the effective GID."
+  (eshell-with-file-attributes-from-name
+    (cl-letf (((symbol-function 'group-gid) (lambda () 1)))
+      (let ((files '("/fake/gid=1" "/fake/gid=2")))
+        (should (equal (eshell-eval-predicate files "G")
+                       '("/fake/gid=1")))))))
+
 (ert-deftest em-pred-test/predicate-links ()
   "Test that \"l\" filters by number of links."
   (eshell-with-file-attributes-from-name
-- 
2.25.1