Subject: bug#69592: 29.2; eshell rm doesn't expand wildcard over TRAMP

From: Jim Porter <[email-masked]>
Date: Wed, 6 Mar 2024 14:10:31 -0800

On 3/6/2024 11:50 AM, Antero Mejr wrote:
> 
> When deleting multiple files with the * wildcard expansion in eshell
> over TRAMP, like this:
> 
> rm /ssh:some-machine:path/*
> 
> nothing is deleted.

Interesting. I didn't realize this case wasn't already covered in 
Eshell. Does the attached patch fix things for you?

From 480802d91185fb863785feb33e3308e6977e7d21 Mon Sep 17 00:00:00 2001
From: Jim Porter <[email-masked]>
Date: Wed, 6 Mar 2024 13:27:07 -0800
Subject: [PATCH] Support expanding Eshell globs for remote file names

* lisp/eshell/em-glob.el (eshell-glob-chars-regexp): New function...
(eshell-glob-regexp): ... use it.
(eshell-glob-p): New function...
(eshell-glob-convert): ... use it, and return the deepest start
directory possible.

* lisp/eshell/esh-util.el (eshell-split-path): Rename to...
(eshell-split-path): ... this, and account for remote file names.

* test/lisp/eshell/em-glob-tests.el
(em-glob-test/convert/current-start-directory)
(em-glob-test/convert/relative-start-directory)
(em-glob-test/convert/absolute-start-directory)
(em-glob-test/convert/remote-start-directory): New tests (bug#69592).
---
 lisp/eshell/em-glob.el            | 34 +++++++++++++--------
 lisp/eshell/esh-util.el           | 51 +++++++++++++++++--------------
 test/lisp/eshell/em-glob-tests.el | 30 ++++++++++++++++++
 3 files changed, 79 insertions(+), 36 deletions(-)

diff --git a/lisp/eshell/em-glob.el b/lisp/eshell/em-glob.el
index b0c3e6e7a11..a358acc970b 100644
--- a/lisp/eshell/em-glob.el
+++ b/lisp/eshell/em-glob.el
@@ -190,6 +190,11 @@ eshell-glob-recursive-alist
   '(("**/" . recurse)
     ("***/" . recurse-symlink)))
 
+(defsubst eshell-glob-chars-regexp ()
+  (or eshell-glob-chars-regexp
+      (setq-local eshell-glob-chars-regexp
+		  (format "[%s]+" (apply 'string eshell-glob-chars-list)))))
+
 (defun eshell-glob-regexp (pattern)
   "Convert glob-pattern PATTERN to a regular expression.
 The basic syntax is:
@@ -210,11 +215,8 @@ eshell-glob-regexp
 resulting regular expression."
   (let ((matched-in-pattern 0)          ; How much of PATTERN handled
 	regexp)
-    (while (string-match
-	    (or eshell-glob-chars-regexp
-                (setq-local eshell-glob-chars-regexp
-		     (format "[%s]+" (apply 'string eshell-glob-chars-list))))
-	    pattern matched-in-pattern)
+    (while (string-match (eshell-glob-chars-regexp)
+                         pattern matched-in-pattern)
       (let* ((op-begin (match-beginning 0))
 	     (op-char (aref pattern op-begin)))
 	(setq regexp
@@ -239,6 +241,9 @@ eshell-glob-regexp
 	    (regexp-quote (substring pattern matched-in-pattern))
 	    "\\'")))
 
+(defun eshell-glob-p (pattern)
+  (string-match (eshell-glob-chars-regexp) pattern))
+
 (defun eshell-glob-convert-1 (glob &optional last)
   "Convert a GLOB matching a single element of a file name to regexps.
 If LAST is non-nil, this glob is the last element of a file name.
@@ -291,14 +296,13 @@ eshell-glob-convert
      symlinks.
 
 3. A boolean indicating whether to match directories only."
-  (let ((globs (eshell-split-path glob))
-        (isdir (eq (aref glob (1- (length glob))) ?/))
+  (let ((globs (eshell-split-filename glob))
+        (isdir (string-suffix-p "/" glob))
         start-dir result last-saw-recursion)
     (if (and (cdr globs)
              (file-name-absolute-p (car globs)))
-        (setq start-dir (car globs)
-              globs (cdr globs))
-      (setq start-dir "."))
+        (setq start-dir (pop globs))
+      (setq start-dir (file-name-as-directory ".")))
     (while globs
       (if-let ((recurse (cdr (assoc (car globs)
                                     eshell-glob-recursive-alist))))
@@ -306,11 +310,15 @@ eshell-glob-convert
               (setcar result recurse)
             (push recurse result)
             (setq last-saw-recursion t))
-        (push (eshell-glob-convert-1 (car globs) (null (cdr globs)))
-              result)
+        (if (or result (eshell-glob-p (car globs)))
+            (push (eshell-glob-convert-1 (car globs) (null (cdr globs)))
+                  result)
+          ;; We haven't seen a glob yet, so instead append to the start
+          ;; directory.
+          (setq start-dir (file-name-concat start-dir (car globs))))
         (setq last-saw-recursion nil))
       (setq globs (cdr globs)))
-    (list (file-name-as-directory start-dir)
+    (list start-dir
           (nreverse result)
           isdir)))
 
diff --git a/lisp/eshell/esh-util.el b/lisp/eshell/esh-util.el
index f0acfecb701..129134814e3 100644
--- a/lisp/eshell/esh-util.el
+++ b/lisp/eshell/esh-util.el
@@ -447,29 +447,34 @@ eshell-parse-colon-path
 	 (parse-colon-path path-env))
       (parse-colon-path path-env))))
 
-(defun eshell-split-path (path)
-  "Split a path into multiple subparts."
-  (let ((len (length path))
-	(i 0) (li 0)
-	parts)
-    (if (and (eshell-under-windows-p)
-	     (> len 2)
-	     (eq (aref path 0) ?/)
-	     (eq (aref path 1) ?/))
-	(setq i 2))
-    (while (< i len)
-      (if (and (eq (aref path i) ?/)
-	       (not (get-text-property i 'escaped path)))
-	  (setq parts (cons (if (= li i) "/"
-			      (substring path li (1+ i))) parts)
-		li (1+ i)))
-      (setq i (1+ i)))
-    (if (< li i)
-	(setq parts (cons (substring path li i) parts)))
-    (if (and (eshell-under-windows-p)
-	     (string-match "\\`[A-Za-z]:\\'" (car (last parts))))
-	(setcar (last parts) (concat (car (last parts)) "/")))
-    (nreverse parts)))
+(defun eshell-split-filename (filename)
+  "Split a FILENAME into a list of file/directory components."
+  (let* ((remote (file-remote-p filename))
+         (filename (file-local-name filename))
+         (len (length filename))
+         (index 0) (curr-start 0)
+         parts)
+    (when (and (eshell-under-windows-p)
+               (string-prefix-p "//" filename))
+      (setq index 2))
+    (while (< index len)
+      (when (and (eq (aref filename index) ?/)
+                 (not (get-text-property index 'escaped filename)))
+        (push (if (= curr-start index) "/"
+                (substring filename curr-start (1+ index)))
+              parts)
+        (setq curr-start (1+ index)))
+      (setq index (1+ index)))
+    (when (< curr-start len)
+      (push (substring filename curr-start) parts))
+    (setq parts (nreverse parts))
+    (when (and (eshell-under-windows-p)
+               (string-match "\\`[A-Za-z]:\\'" (car parts)))
+      (setcar parts (concat (car parts) "/")))
+    (if remote (cons remote parts) parts)))
+
+(define-obsolete-function-alias 'eshell-split-path
+  'eshell-split-filename "30.1")
 
 (defun eshell-to-flat-string (value)
   "Make value a string.  If separated by newlines change them to spaces."
diff --git a/test/lisp/eshell/em-glob-tests.el b/test/lisp/eshell/em-glob-tests.el
index 6d922666ea3..fc460a59eed 100644
--- a/test/lisp/eshell/em-glob-tests.el
+++ b/test/lisp/eshell/em-glob-tests.el
@@ -61,6 +61,9 @@ with-fake-files
 
 ;;; Tests:
 
+
+;; Glob expansion
+
 (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."
@@ -115,6 +118,33 @@ em-glob-test/expand/explicitly-listify-results
           (eshell-command-result-equal "list ${listify *.no}"
                                        '(("*.no"))))))))
 
+
+;; Glob conversion
+
+(ert-deftest em-glob-test/convert/current-start-directory ()
+  "Test converting a glob starting in the current directory."
+  (should (equal (eshell-glob-convert "*.el")
+                 '("./" (("\\`.*\\.el\\'" . "\\`\\.")) nil))))
+
+(ert-deftest em-glob-test/convert/relative-start-directory ()
+  "Test converting a glob starting in a relative directory."
+  (should (equal (eshell-glob-convert "some/where/*.el")
+                 '("./some/where/" (("\\`.*\\.el\\'" . "\\`\\.")) nil))))
+
+(ert-deftest em-glob-test/convert/absolute-start-directory ()
+  "Test converting a glob starting in an absolute directory."
+  (should (equal (eshell-glob-convert "/some/where/*.el")
+                 '("/some/where/" (("\\`.*\\.el\\'" . "\\`\\.")) nil))))
+
+(ert-deftest em-glob-test/convert/remote-start-directory ()
+  "Test converting a glob starting in a remote directory."
+  (should (equal (eshell-glob-convert "/ssh:nowhere.invalid:some/where/*.el")
+                 '("/ssh:nowhere.invalid:/some/where/"
+                   (("\\`.*\\.el\\'" . "\\`\\.")) nil))))
+
+
+;; Glob matching
+
 (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")
-- 
2.25.1