bug#53371: 28.0.90; eshell completion error when trying to complete buffer name, args-out-of-range

[Jim Porter <jporterbugs@gmail.com>]

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

On 1/19/2022 9:35 AM, Daniel Mendler wrote:
> 1. Start emacs -Q
> 2. M-x eshell
> 3. Enter "ls > #<" for buffer redirection.
> 4. Pressing TAB results in an args-out-of-range error, see below.
> 
> It seems that completing buffer names is not supported. This feature
> would be nice to have. However in any case, no args-out-of-range error
> should occur.

Here's a patch for this, with a few tests. Note that it depends on my 
patches in bug#61221, so it can't merge quite yet.

[-- Attachment #2: 0001-Add-support-for-completing-special-references-e.g.-b.patch --]
[-- Type: text/plain, Size: 11110 bytes --]

From 5099ffc8ee4ff1f821fa7da222351a6c40dd64a9 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Tue, 24 Jan 2023 21:22:06 -0800
Subject: [PATCH] Add support for completing special references (e.g. buffers)
 in Eshell

* lisp/eshell/em-cmpl.el (eshell-complete-parse-arguments): Handle
special references.

* lisp/eshell/em-arg.el (eshell-parse-special-reference): Ensure point
is just after the "#<" when incomplete, and handle backslash escapes
more thoroughly.
(eshell-complete-special-reference)
(eshell-complete-special-ref--exit): New functions.

* test/lisp/eshell/esh-arg-tests.el
(esh-arg-test/special-reference/default)
(esh-arg-test/special-reference/buffer)
(esh-arg-test/special-reference/special):
* test/lisp/eshell/em-cmpl-tests.el
(em-cmpl-test/special-ref-completion/type)
(em-cmpl-test/special-ref-completion/implicit-buffer)
(em-cmpl-test/special-ref-completion/buffer): New tests.
---
 lisp/eshell/em-cmpl.el            | 10 +++--
 lisp/eshell/esh-arg.el            | 63 +++++++++++++++++++++++++++----
 test/lisp/eshell/em-cmpl-tests.el | 40 ++++++++++++++++++++
 test/lisp/eshell/esh-arg-tests.el | 30 +++++++++++++++
 4 files changed, 131 insertions(+), 12 deletions(-)

diff --git a/lisp/eshell/em-cmpl.el b/lisp/eshell/em-cmpl.el
index 380ecd0b91d..b2a69d70f3a 100644
--- a/lisp/eshell/em-cmpl.el
+++ b/lisp/eshell/em-cmpl.el
@@ -319,7 +319,7 @@ eshell-complete-parse-arguments
     (eshell--pcomplete-insert-tab))
   (let ((end (point-marker))
 	(begin (save-excursion (beginning-of-line) (point)))
-	args posns delim)
+	args posns delim incomplete-arg)
     (when (and pcomplete-allow-modifications
 	       (memq this-command '(pcomplete-expand
 			            pcomplete-expand-and-complete)))
@@ -334,10 +334,11 @@ eshell-complete-parse-arguments
         (cond ((member (car delim) '("{" "${" "$<"))
 	       (setq begin (1+ (cadr delim))
 		     args (eshell-parse-arguments begin end)))
-              ((member (car delim) '("$'" "$\""))
+              ((member (car delim) '("$'" "$\"" "#<"))
                ;; Add the (incomplete) argument to our arguments, and
                ;; note its position.
-               (setq args (append (nth 2 delim) (list (car delim))))
+               (setq args (append (nth 2 delim) (list (car delim)))
+                     incomplete-arg t)
                (push (- (nth 1 delim) 2) posns))
               ((member (car delim) '("(" "$("))
 	       (throw 'pcompleted (elisp-completion-at-point)))
@@ -364,7 +365,8 @@ eshell-complete-parse-arguments
 	(setq args (nthcdr (1+ new-start) args)
 	      posns (nthcdr (1+ new-start) posns))))
     (cl-assert (= (length args) (length posns)))
-    (when (and args (eq (char-syntax (char-before end)) ? )
+    (when (and args (not incomplete-arg)
+               (eq (char-syntax (char-before end)) ? )
 	       (not (eq (char-before (1- end)) ?\\)))
       (nconc args (list ""))
       (nconc posns (list (point))))
diff --git a/lisp/eshell/esh-arg.el b/lisp/eshell/esh-arg.el
index cb0b2e0938c..6acc6c812d3 100644
--- a/lisp/eshell/esh-arg.el
+++ b/lisp/eshell/esh-arg.el
@@ -28,6 +28,9 @@
 ;;; Code:
 
 (require 'esh-util)
+(require 'esh-module)
+
+(require 'pcomplete)
 
 (eval-when-compile
   (require 'cl-lib))
@@ -175,7 +178,11 @@ eshell-arg-initialize
   "Initialize the argument parsing code."
   (eshell-arg-mode)
   (setq-local eshell-inside-quote-regexp nil)
-  (setq-local eshell-outside-quote-regexp nil))
+  (setq-local eshell-outside-quote-regexp nil)
+
+  (when (eshell-using-module 'eshell-cmpl)
+    (add-hook 'pcomplete-try-first-hook
+              #'eshell-complete-special-reference nil t)))
 
 (defun eshell-insert-buffer-name (buffer-name)
   "Insert BUFFER-NAME into the current buffer at point."
@@ -506,21 +513,28 @@ eshell-parse-special-reference
 \"buffer\"."
   (when (and (not eshell-current-argument)
              (not eshell-current-quoted)
-             (looking-at "#<\\(\\(buffer\\|process\\)\\s-\\)?"))
+             (looking-at (rx "#<" (? (group (or "buffer" "process"))
+                                     space))))
     (let ((here (point)))
       (goto-char (match-end 0)) ;; Go to the end of the match.
-      (let ((buffer-p (if (match-string 1)
-                          (string= (match-string 2) "buffer")
+      (let ((buffer-p (if (match-beginning 1)
+                          (equal (match-string 1) "buffer")
                         t)) ;; buffer-p is non-nil by default.
             (end (eshell-find-delimiter ?\< ?\>)))
         (when (not end)
+          (when (match-beginning 1)
+            (goto-char (match-beginning 1)))
           (throw 'eshell-incomplete "#<"))
         (if (eshell-arg-delimiter (1+ end))
             (prog1
-                (list (if buffer-p 'get-buffer-create 'get-process)
-                      (replace-regexp-in-string
-                       (rx "\\" (group (or "\\" "<" ">"))) "\\1"
-                       (buffer-substring-no-properties (point) end)))
+                (list (if buffer-p #'get-buffer-create #'get-process)
+                      ;; FIXME: We should probably parse this as a
+                      ;; real Eshell argument so that we get the
+                      ;; benefits of quoting, variable-expansion, etc.
+                      (string-trim-right
+                       (replace-regexp-in-string
+                        (rx "\\" (group anychar)) "\\1"
+                        (buffer-substring-no-properties (point) end))))
               (goto-char (1+ end)))
           (ignore (goto-char here)))))))
 
@@ -574,5 +588,38 @@ eshell-prepare-splice
     (when splicep
       grouped-args)))
 
+;;;_* Special ref completion
+
+(defun eshell-complete-special-reference ()
+  "If there is a special reference, complete it."
+  (let ((arg (pcomplete-actual-arg)))
+    (when (string-match
+           (rx string-start
+               "#<" (? (group (or "buffer" "process")) space)
+               (group (* anychar))
+               string-end)
+           arg)
+      (let ((all-results (if (equal (match-string 1 arg) "process")
+                             (mapcar #'process-name (process-list))
+                           (mapcar #'buffer-name (buffer-list)))))
+        (unless (match-beginning 1)
+          ;; Include the special reference types as completion options.
+          (setq all-results (append '("buffer" "process") all-results)))
+        (setq pcomplete-stub (replace-regexp-in-string
+                              (rx "\\" (group anychar)) "\\1"
+                              (substring arg (match-beginning 2)))
+              pcomplete-exit-function #'eshell-complete-special-ref--exit)
+        (throw 'pcomplete-completions
+               (all-completions pcomplete-stub all-results))))))
+
+(defun eshell-complete-special-ref--exit (value status)
+  "An exit function for completing Eshell special references.
+STATUS is a symbol representing the state of the completion."
+  (when (and (eq status 'finished)
+             (not (member value '("buffer" "process"))))
+    (if (looking-at ">")
+        (goto-char (match-end 0))
+      (insert ">"))))
+
 (provide 'esh-arg)
 ;;; esh-arg.el ends here
diff --git a/test/lisp/eshell/em-cmpl-tests.el b/test/lisp/eshell/em-cmpl-tests.el
index ecab7332822..abc39721d9b 100644
--- a/test/lisp/eshell/em-cmpl-tests.el
+++ b/test/lisp/eshell/em-cmpl-tests.el
@@ -176,6 +176,46 @@ em-cmpl-test/lisp-function-completion
    (should (equal (eshell-insert-and-complete "echo (eshell/ech")
                   "echo (eshell/echo"))))
 
+(ert-deftest em-cmpl-test/special-ref-completion/type ()
+  "Test completion of the start of special references like \"#<buffer\".
+See <lisp/eshell/esh-arg.el>."
+  (with-temp-eshell
+   (should (equal (eshell-insert-and-complete "echo hi > #<buf")
+                  "echo hi > #<buffer ")))
+  (with-temp-eshell
+   (should (equal (eshell-insert-and-complete "echo hi > #<proc")
+                  "echo hi > #<process "))))
+
+(ert-deftest em-cmpl-test/special-ref-completion/implicit-buffer ()
+  "Test completion of special references like \"#<buf>\".
+See <lisp/eshell/esh-arg.el>."
+  (let (bufname)
+    (with-temp-buffer
+      (setq bufname (rename-buffer "my-buffer" t))
+      (with-temp-eshell
+       (should (equal (eshell-insert-and-complete "echo hi > #<my-buf")
+                      (format "echo hi > #<%s> " bufname))))
+      (setq bufname (rename-buffer "another buffer" t))
+      (with-temp-eshell
+       (should (equal (eshell-insert-and-complete "echo hi > #<anoth")
+                      (format "echo hi > #<%s> "
+                              (string-replace " " "\\ " bufname))))))))
+
+(ert-deftest em-cmpl-test/special-ref-completion/buffer ()
+  "Test completion of special references like \"#<buffer buf>\".
+See <lisp/eshell/esh-arg.el>."
+  (let (bufname)
+    (with-temp-buffer
+      (setq bufname (rename-buffer "my-buffer" t))
+      (with-temp-eshell
+       (should (equal (eshell-insert-and-complete "echo hi > #<buffer my-buf")
+                      (format "echo hi > #<buffer %s> " bufname))))
+      (setq bufname (rename-buffer "another buffer" t))
+      (with-temp-eshell
+       (should (equal (eshell-insert-and-complete "echo hi > #<buffer anoth")
+                      (format "echo hi > #<buffer %s> "
+                              (string-replace " " "\\ " bufname))))))))
+
 (ert-deftest em-cmpl-test/variable-ref-completion ()
   "Test completion of variable references like \"$var\".
 See <lisp/eshell/esh-var.el>."
diff --git a/test/lisp/eshell/esh-arg-tests.el b/test/lisp/eshell/esh-arg-tests.el
index 918ad3a949f..c883db3907f 100644
--- a/test/lisp/eshell/esh-arg-tests.el
+++ b/test/lisp/eshell/esh-arg-tests.el
@@ -102,4 +102,34 @@ esh-arg-test/escape-quoted/newline
    (eshell-match-command-output "echo \"hi\\\nthere\""
                                 "hithere\n")))
 
+(ert-deftest esh-arg-test/special-reference/default ()
+  "Test that \"#<buf>\" refers to the buffer \"buf\"."
+  (with-temp-buffer
+    (rename-buffer "my-buffer" t)
+    (eshell-command-result-equal
+     (format "echo #<%s>" (buffer-name))
+     (current-buffer))))
+
+(ert-deftest esh-arg-test/special-reference/buffer ()
+  "Test that \"#<buffer buf>\" refers to the buffer \"buf\"."
+  (with-temp-buffer
+    (rename-buffer "my-buffer" t)
+    (eshell-command-result-equal
+     (format "echo #<buffer %s>" (buffer-name))
+     (current-buffer))))
+
+(ert-deftest esh-arg-test/special-reference/special ()
+  "Test that \"#<...>\" works correctly when escaping special characters."
+  (with-temp-buffer
+    (rename-buffer "<my buffer>" t)
+    (let ((escaped-bufname (replace-regexp-in-string
+                            (rx (group (or "\\" "<" ">" space))) "\\\\\\1"
+                            (buffer-name))))
+      (eshell-command-result-equal
+       (format "echo #<%s>" escaped-bufname)
+       (current-buffer))
+      (eshell-command-result-equal
+       (format "echo #<buffer %s>" escaped-bufname)
+       (current-buffer)))))
+
 ;; esh-arg-tests.el ends here
-- 
2.25.1