bug#65358: 30.0.50 [PATCH]; Add support for splicing Eshell globs in-place

[Jim Porter <jporterbugs@gmail.com>]

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

One odd thing about Eshell is that when you expand a glob, it expands as 
a sublist within your argument list. In practice, that usually works out 
ok, since external commands (and some Eshell built-ins) will flatten the 
list of arguments before processing them.

However, this can cause issues when calling Lisp commands. For example, 
if I have a single patch file in my current directory, I might try to 
refer to it via "*.patch". This works fine for a command like "cat", but 
not for "find-file":

   ~ $ find-file *.patch
   Wrong type argument: stringp, 
("0001-Allow-splicing-Eshell-globs-in-place.patch")

With the attached patch, you can set 'eshell-glob-splice-results' to t 
and then the above command will work as expected.

I'm tempted to make this new behavior the default, since it smooths over 
the differences between Emacs Lisp functions and external commands in 
Eshell (when using the command form, that is). However, at least for 
now, I think it would be best to leave this off by default. Once any 
bugs have been worked out and people have had the chance to try it out, 
when we could consider changing the default one day.

[-- Attachment #2: 0001-Allow-splicing-Eshell-globs-in-place.patch --]
[-- Type: text/plain, Size: 12403 bytes --]

From d6a244abd007548721527334804f491eeca03459 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Thu, 17 Aug 2023 12:23:26 -0700
Subject: [PATCH] Allow splicing Eshell globs in-place

This means that Eshell globs can now expand the same way as if the
user had typed each matching file individually.

* lisp/eshell/em-glob.el (eshell-glob-splice-results): New option.
(eshell-no-command-globbing, eshell-add-glob-modifier): Handle spliced
globs.
(eshell-extended-glob): Always return a list when splicing.

* lisp/eshell/em-pred.el (eshell-parse-arg-modifier): Ensure
'eshell-splice-args' is always at the end of the list of modifiers if
present.

* test/lisp/eshell/em-glob-tests.el
(em-glob-test/expand/splice-results)
(em-glob-test/expand/no-splice-results)
(em-glob-test/expand/explicitly-splice-results)
(em-glob-test/expand/explicitly-listify-results): New tests.
(em-glob-test/no-matches): Check result when
'eshell-glob-splice-results' is nil/non-nil.

* doc/misc/eshell.texi (Arguments): Expand explanation about argument
flattening.
(Globbing): Document splicing behavior of globs.

* etc/NEWS: Announce this change.
---
 doc/misc/eshell.texi              | 31 +++++++++------
 etc/NEWS                          |  7 ++++
 lisp/eshell/em-glob.el            | 22 +++++++++--
 lisp/eshell/em-pred.el            | 29 +++++++++-----
 test/lisp/eshell/em-glob-tests.el | 64 +++++++++++++++++++++++++++++++
 5 files changed, 128 insertions(+), 25 deletions(-)

diff --git a/doc/misc/eshell.texi b/doc/misc/eshell.texi
index 6890728a81d..59c07457158 100644
--- a/doc/misc/eshell.texi
+++ b/doc/misc/eshell.texi
@@ -317,9 +317,10 @@ Arguments
 (1 2 3)
 @end example
 
-Additionally, many built-in Eshell commands (@pxref{Built-ins}) will
-flatten the arguments they receive, so passing a list as an argument
-will ``spread'' the elements into multiple arguments:
+When calling external commands (and many built-in Eshell commands,
+too) Eshell will flatten the arguments the command receives, so
+passing a list as an argument will ``spread'' the elements into
+multiple arguments:
 
 @example
 ~ $ printnl (list 1 2) 3
@@ -1466,18 +1467,28 @@ Dollars Expansion
 
 @node Globbing
 @section Globbing
-@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
+@vindex eshell-glob-case-insensitive
+Globs are case sensitive by default, 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
-@code{eshell-glob} (@pxref{Easy Customization, , , emacs, The GNU
-Emacs Manual}).
+@code{eshell-glob-case-insensitive} option.
+
+@vindex eshell-glob-splice-results
+By default, Eshell expands the results of a glob as a sublist into the
+list of arguments.  You can change this to splice the results in-place
+by setting @code{eshell-glob-splice-results} to a non-@code{nil}
+value.  If you want to splice a glob in-place for just one use, you
+can use a subcommand form like @samp{$@@@{listify @var{my-glob}@}}.
+(Conversely, you can explicitly expand a glob as a sublist via
+@samp{$@{listify @var{my-glob}@}}.)
+
+You can further customize the syntax and behavior of globbing in
+Eshell via the Customize group @code{eshell-glob} (@pxref{Easy
+Customization, , , emacs, The GNU Emacs Manual}).
 
 @table @samp
 
@@ -2386,8 +2397,6 @@ Bugs and ideas
 This would be so that if a Lisp function calls @code{print}, everything
 will happen as it should (albeit slowly).
 
-@item If a globbing pattern returns one match, should it be a list?
-
 @item Make sure syntax table is correct in Eshell mode
 
 So that @kbd{M-@key{DEL}} acts in a predictable manner, etc.
diff --git a/etc/NEWS b/etc/NEWS
index 966d1f8c292..b6dcad24cff 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -312,6 +312,13 @@ of arguments into a command, such as when defining aliases.  For more
 information, see the "(eshell) Dollars Expansion" node in the Eshell
 manual.
 
++++
+*** You can now splice Eshell globs in-place into argument lists.
+By setting 'eshell-glob-splice-results' to a non-nil value, Eshell
+will expand glob results in-place as if you had typed each matching
+file name individually.  For more information, see the "(eshell)
+Globbing" node in the Eshell manual.
+
 +++
 *** Eshell now supports negative numbers and ranges for indices.
 Now, you can retrieve the last element of a list with '$my-list[-1]'
diff --git a/lisp/eshell/em-glob.el b/lisp/eshell/em-glob.el
index d00f8c93cd1..1141b673e97 100644
--- a/lisp/eshell/em-glob.el
+++ b/lisp/eshell/em-glob.el
@@ -69,6 +69,15 @@ eshell-glob-load-hook
   :type 'hook
   :group 'eshell-glob)
 
+(defcustom eshell-glob-splice-results nil
+  "If non-nil, the results of glob patterns will be spliced in-place.
+When splicing, the resulting command is as though the user typed
+each result individually.  Otherwise, the glob results a single
+argument as a list."
+  :version "30.1"
+  :type 'boolean
+  :group 'eshell-glob)
+
 (defcustom eshell-glob-include-dot-files nil
   "If non-nil, glob patterns will match files beginning with a dot."
   :type 'boolean
@@ -139,12 +148,15 @@ eshell-glob-initialize
 (defun eshell-no-command-globbing (terms)
   "Don't glob the command argument.  Reflect this by modifying TERMS."
   (ignore
-   (when (and (listp (car terms))
-	      (eq (caar terms) 'eshell-extended-glob))
-     (setcar terms (cadr (car terms))))))
+   (pcase (car terms)
+     ((or `(eshell-extended-glob ,term)
+          `(eshell-splice-args (eshell-extended-glob ,term)))
+      (setcar terms term)))))
 
 (defun eshell-add-glob-modifier ()
   "Add `eshell-extended-glob' to the argument modifier list."
+  (when eshell-glob-splice-results
+    (add-to-list 'eshell-current-modifiers 'eshell-splice-args t))
   (add-to-list 'eshell-current-modifiers 'eshell-extended-glob))
 
 (defun eshell-parse-glob-chars ()
@@ -326,7 +338,9 @@ eshell-extended-glob
     (or (and eshell-glob-matches (sort eshell-glob-matches #'string<))
 	(if eshell-error-if-no-glob
 	    (error "No matches found: %s" glob)
-	  glob))))
+          (if eshell-glob-splice-results
+              (list glob)
+            glob)))))
 
 ;; FIXME does this really need to abuse eshell-glob-matches, message-shown?
 (defun eshell-glob-entries (path globs only-dirs)
diff --git a/lisp/eshell/em-pred.el b/lisp/eshell/em-pred.el
index bfb0dad60ef..1d67f1af990 100644
--- a/lisp/eshell/em-pred.el
+++ b/lisp/eshell/em-pred.el
@@ -301,16 +301,25 @@ eshell-parse-arg-modifier
                    (modifiers (eshell-parse-modifiers))
 		   (preds (car modifiers))
 		   (mods (cdr modifiers)))
-	      (if (or preds mods)
-		  ;; has to go at the end, which is only natural since
-		  ;; syntactically it can only occur at the end
-		  (setq eshell-current-modifiers
-			(append
-			 eshell-current-modifiers
-			 (list
-			  (lambda (lst)
-			    (eshell-apply-modifiers
-			     lst preds mods modifier-string))))))))
+              (when (or preds mods)
+                ;; Has to go at the end, which is only natural since
+                ;; syntactically it can only occur at the end.
+                (setq eshell-current-modifiers
+                      (append
+                       eshell-current-modifiers
+                       (list
+                        (lambda (lst)
+                          (eshell-apply-modifiers
+                           lst preds mods modifier-string)))))
+                (when (memq 'eshell-splice-args eshell-current-modifiers)
+                  ;; If splicing results, ensure that
+                  ;; `eshell-splice-args' is the last modifier.
+                  ;; Eshell's command parsing can't handle it anywhere
+                  ;; else.
+                  (setq eshell-current-modifiers
+                        (append (delq 'eshell-splice-args
+                                      eshell-current-modifiers)
+                                (list 'eshell-splice-args)))))))
 	  (goto-char (1+ end))
 	  (eshell-finish-arg))))))
 
diff --git a/test/lisp/eshell/em-glob-tests.el b/test/lisp/eshell/em-glob-tests.el
index c33af88a374..6e07225657c 100644
--- a/test/lisp/eshell/em-glob-tests.el
+++ b/test/lisp/eshell/em-glob-tests.el
@@ -26,6 +26,13 @@
 (require 'ert)
 (require 'em-glob)
 
+(require 'eshell-tests-helpers
+         (expand-file-name "eshell-tests-helpers"
+                           (file-name-directory (or load-file-name
+                                                    default-directory))))
+
+(defvar eshell-prefer-lisp-functions)
+
 (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
@@ -54,6 +61,60 @@ with-fake-files
 
 ;;; Tests:
 
+(ert-deftest em-glob-test/expand/splice-results ()
+  "Test that globs are spliced into the argument list when
+`eshell-glob-splice-results' is non-nil."
+  (let ((eshell-prefer-lisp-functions t)
+        (eshell-glob-splice-results t))
+    (with-fake-files '("a.el" "b.el" "c.txt")
+      ;; Ensure the default expansion splices the glob.
+      (eshell-command-result-equal "list *.el" '("a.el" "b.el"))
+      (eshell-command-result-equal "list *.txt" '("c.txt"))
+      (eshell-command-result-equal "list *.no" '("*.no")))))
+
+(ert-deftest em-glob-test/expand/no-splice-results ()
+  "Test that globs are treated as lists when
+`eshell-glob-splice-results' is nil."
+  (let ((eshell-prefer-lisp-functions t)
+        (eshell-glob-splice-results nil))
+    (with-fake-files '("a.el" "b.el" "c.txt")
+      ;; Ensure the default expansion splices the glob.
+      (eshell-command-result-equal "list *.el" '(("a.el" "b.el")))
+      (eshell-command-result-equal "list *.txt" '(("c.txt")))
+      ;; The no-matches case is special here: the glob is just the
+      ;; string, not the list of results.
+      (eshell-command-result-equal "list *.no" '("*.no")))))
+
+(ert-deftest em-glob-test/expand/explicitly-splice-results ()
+  "Test explicitly splicing globs works the same no matter the
+value of `eshell-glob-splice-results'."
+  (let ((eshell-prefer-lisp-functions t))
+    (dolist (eshell-glob-splice-results '(nil t))
+      (ert-info ((format "eshell-glob-splice-results: %s"
+                         eshell-glob-splice-results))
+        (with-fake-files '("a.el" "b.el" "c.txt")
+          (eshell-command-result-equal "list $@{listify *.el}"
+                                       '("a.el" "b.el"))
+          (eshell-command-result-equal "list $@{listify *.txt}"
+                                       '("c.txt"))
+          (eshell-command-result-equal "list $@{listify *.no}"
+                                       '("*.no")))))))
+
+(ert-deftest em-glob-test/expand/explicitly-listify-results ()
+  "Test explicitly listifying globs works the same no matter the
+value of `eshell-glob-splice-results'."
+  (let ((eshell-prefer-lisp-functions t))
+    (dolist (eshell-glob-splice-results '(nil t))
+      (ert-info ((format "eshell-glob-splice-results: %s"
+                         eshell-glob-splice-results))
+        (with-fake-files '("a.el" "b.el" "c.txt")
+          (eshell-command-result-equal "list ${listify *.el}"
+                                       '(("a.el" "b.el")))
+          (eshell-command-result-equal "list ${listify *.txt}"
+                                       '(("c.txt")))
+          (eshell-command-result-equal "list ${listify *.no}"
+                                       '(("*.no"))))))))
+
 (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")
@@ -191,6 +252,9 @@ em-glob-test/no-matches
   (with-fake-files '("foo.el" "bar.el")
     (should (equal (eshell-extended-glob "*.txt")
                    "*.txt"))
+    (let ((eshell-glob-splice-results t))
+      (should (equal (eshell-extended-glob "*.txt")
+                     '("*.txt"))))
     (let ((eshell-error-if-no-glob t))
       (should-error (eshell-extended-glob "*.txt")))))
 
-- 
2.25.1