all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#54470: 29.0.50; [PATCH] Add documentation/tests for Eshell argument expansion
@ 2022-03-20  1:34 Jim Porter
  2022-03-20  7:05 ` Eli Zaretskii
  0 siblings, 1 reply; 14+ messages in thread
From: Jim Porter @ 2022-03-20  1:34 UTC (permalink / raw)
  To: 54470

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

Eshell supports a few ways of expanding and manipulating arguments: 
globs (which most people are likely familiar with from other shells), as 
well as predicates (which let you filter lists of file names based on 
the files' properties) and modifiers (which perform common 
transformations, mostly on lists). However, these lack unit tests and 
are currently only documented in the manual as follows:

   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. See Filename Generation in
   The Z Shell Manual, for full syntax.

As the manual says, the syntax is "similar"; it's not actually the same. 
It also doesn't mention argument modifiers, which are related to 
predicates, but let you do different things. I think it would be best to 
fully-document the syntax so that the Eshell-specific bits are clear. 
Attached are some patches to add documentation and tests for this.

For globbing, I only added tests/documentation, but for predicates and 
modifiers, I found a few bugs as well. I'll describe each of them:

1. The "is a socket" predicate conflicts with the "is setuid" predicate 
(both are `s'), meaning that it's impossible to filter on the setuid 
bit. I've updated the "is a socket" predicate to be `=', matching zsh.

2. The "join" (`j') modifier was documented as joining a list and 
separating the element by a space, but it actually joined the list with 
no separator. It now behaves according to the documentation.

3. The "eval again" (`E') modifier didn't work; it called 
`eshell-parse-argument' with one argument, but that function takes no 
args. I've fixed this, though I had to make an educated guess on what 
the behavior should be. I chose the behavior closest to what the 
previous implementation looked like it was trying to do: re-evaluate the 
value as though it were a single argument (another option would be to 
re-evaluate as though it were a full shell command, i.e. to invoke a 
program). I've documented the behavior in the manual, so hopefully that 
will explain how it works.

Finally, I added support for the "is effective gid" predicate (`G'). 
That was already there, but commented out. It just needed uncommenting 
and the uid parts renamed to gid. I'm not sure if there was some reason 
for it to be commented out, but it works fine in my tests.

Hopefully the documentation I've added is structured/worded reasonably 
well. I did my best to follow existing conventions despite the 
very-different syntax (especially for predicates/modifiers), and tried 
to give a reasonable level of detail for all the options. If there's 
anything that's confusing or needs expanded, just let me know and I'll 
try to improve it.

Finally, I'm not sure if any of the behavior changes should be 
documented in NEWS. `G' ("is effective gid") is a small new feature, and 
the change from `s' to `=' for "is a socket" and the change for `j' 
(join) are technically incompatible changes, although maybe they're 
obscure enough that they don't need NEWS entries. I'll do whatever 
others think is best here.

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

From 6ecd300a0bdddf369c8a3672e70fec175ccb8a0d 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.

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

* doc/misc/eshell.texi (Globbing): Document pattern-based globs.
---
 doc/misc/eshell.texi              |  80 ++++++++++++--
 lisp/eshell/em-glob.el            |  12 +--
 test/lisp/eshell/em-glob-tests.el | 171 ++++++++++++++++++++++++++++++
 3 files changed, 247 insertions(+), 16 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..4a1fdcf8ff 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1089,15 +1089,77 @@ 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''.
+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.  To customize the syntax and behavior of globbing
+in Eshell see the Customize@footnote{@xref{Easy Customization, , ,
+emacs, The GNU Emacs Manual}.} group ``eshell-glob''.
+
+@table @code
+
+@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 any number of subdirectories in a file name, including zero.
+For example, @samp{**/foo.el} matches @file{foo.el},
+@file{bar/foo.el}, @file{bar/baz/foo.el}, etc.
+
+@item ***
+As @code{**}, but follows symlinks as well.
+
+@item [ @dots{} ]
+Defines a @dfn{character set} (@pxref{Regexps, , , emacs, The GNU
+Emacs Manual}).  A character set matches any character between
+@samp{[} and @samp{]}; 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.
+
+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.
+
+@item [^ @dots{} ]
+Defines a @dfn{complemented character set}.  This behaves just like a
+character set, but matches any character @emph{except} the ones
+specified.
+
+@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..8c0d8dfcd7 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
diff --git a/test/lisp/eshell/em-glob-tests.el b/test/lisp/eshell/em-glob-tests.el
new file mode 100644
index 0000000000..1063ba2423
--- /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: 36718 bytes --]

From 23e5f6b2927fe30f77cae95ee5116ad316be09ed 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-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              | 221 +++++++++++++
 lisp/eshell/em-pred.el            |  25 +-
 lisp/eshell/esh-cmd.el            |   8 +
 test/lisp/eshell/em-pred-tests.el | 520 ++++++++++++++++++++++++++++++
 4 files changed, 759 insertions(+), 15 deletions(-)
 create mode 100644 test/lisp/eshell/em-pred-tests.el

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 4a1fdcf8ff..36ab871418 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1002,6 +1002,7 @@ Expansion
 @menu
 * Dollars Expansion::
 * Globbing::
+* Argument Predication::
 @end menu
 
 @node Dollars Expansion
@@ -1161,6 +1162,226 @@ Globbing
 
 @end table
 
+@node Argument Predication
+@section Argument Predication and 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{( @dots{} )} after
+any list argument, where @code{@dots{}} 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.
+
+To customize the syntax and behavior of predicates and modifiers in
+Eshell see the Customize@footnote{@xref{Easy Customization, , , emacs,
+The GNU Emacs Manual}.} group ``eshell-pred''.
+
+@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.
+
+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.
+
+Eshell supports the following argument predicates:
+
+@table @asis
+
+@item @code{/}
+Matches directories.
+
+@item @code{.} (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
+
+@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'').
+
+@item t
+Treating the value as a file name, gets the base name (the ``tail'').
+
+@item e
+Treating the value as a file name, gets the file extension.
+
+@item r
+Treating the value as a file name, gets the file name sans extension.
+
+@item q
+Marks that the value should be interpreted by Eshell literally.
+
+@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 by the regular expression @var{pattern}.  If
+@var{pattern} is omitted, split on spaces.
+
+@item j
+@item j/@var{delim}/
+Joins a list of values, separated by the string @var{delim}.  If
+@var{delim} is omitted, use a single space as the delimiter.
+
+@item o
+Sorts a list of strings in ascending lexicographic order.
+
+@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..e75da91cf6 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")
@@ -568,7 +563,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..f746aa3da8
--- /dev/null
+++ b/test/lisp/eshell/em-pred-tests.el
@@ -0,0 +1,520 @@
+;;; 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" "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: 3187 bytes --]

From a39c5387bd0f213d0c4b5fe19e1844c9dd2bb363 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 36ab871418..c1f9a2773d 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -1253,6 +1253,9 @@ Argument Predication
 @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 e75da91cf6..8d26b290d2 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 f746aa3da8..9c4e975dd0 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


^ permalink raw reply related	[flat|nested] 14+ messages in thread
end of thread, other threads:[~2022-04-17 18:38 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-03-20  1:34 bug#54470: 29.0.50; [PATCH] Add documentation/tests for Eshell argument expansion Jim Porter
2022-03-20  7:05 ` Eli Zaretskii
2022-03-20 20:57   ` Jim Porter
2022-03-28  2:29     ` Jim Porter
2022-03-30  4:47     ` Jim Porter
2022-03-31  7:19     ` Eli Zaretskii
2022-04-01  4:11       ` Richard Stallman
2022-04-02  5:10         ` Jim Porter
2022-04-15 12:56           ` Eli Zaretskii
2022-04-16  4:57             ` Jim Porter
2022-04-16 10:30               ` Eli Zaretskii
2022-04-16 17:14                 ` Jim Porter
2022-04-17  7:32                   ` Eli Zaretskii
2022-04-17 18:38                     ` Jim Porter

Code repositories for project(s) associated with this external index

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.