Subject: bug#61221: closed (Re: bug#61221: 30.0.50; [PATCH] Support completion of quoted variable refs in Eshell)

From: [email-masked] (GNU bug Tracking System)
Date: Thu, 23 Feb 2023 22:13:02 +0000

Your bug report

#61221: 30.0.50; [PATCH] Support completion of quoted variable refs in Eshell

which was filed against the emacs package, has been closed.

The explanation is attached below, along with your original report.
If you require more details, please reply to [email-masked].

-- 
61221: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=61221
GNU Bug Tracking System
Contact [email-masked] with problems

Subject: Re: bug#61221: 30.0.50; [PATCH] Support completion of quoted variable refs in Eshell
From: Jim Porter <[email-masked]>
To: Stefan Monnier <[email-masked]>
Cc: [email-masked]
Date: Thu, 23 Feb 2023 14:11:54 -0800
List-Id: <debbugs-submit.debbugs.gnu.org>

On 2/23/2023 10:02 AM, Stefan Monnier via Bug reports for GNU Emacs, the 
Swiss army knife of text editors wrote:
> The patches look good, feel free to push, AFAIC.

Thanks, merged as 9d48c9844b.

>> +    (mapcar (lambda (x) (car x))
> 
> Aka (mapcar #'car

Fixed.

>> +      (setq pcomplete-stub (substring arg (match-beginning 1))
>> +            delimiter (match-string 2 arg))
> 
> You could let-bind `delimiter` here instead of let-binding it earlier
> and then `setq`ing it here.  Better for karma and marginally more
> efficient (avoids the creation of a `cons` cell to contain the value of
> the var).

Done.

>> +     (append (eshell-envvar-names)
>> +             (all-completions argname obarray 'boundp))
>> +     #'string-lessp)))
> 
> Since you use #' for `string-lessp`, it would make sense to use #' for
> `boundp` as well :-)

Fixed.

>> +                   ('lambda               ; test-completion
>> +                     (let ((result (test-completion string names pred)))
>> +                       (if (eq result t) string result)))
> 
> Hmm... why not just always return `result`?

As I understand it, returning 't' means "there is just one matching 
completion, and the match is exact"[1], but in this case, that's not 
really true: after completing "~user/" there are still more matching 
completions (the contents of the user's home directory).

This is really just trying to match what happens when calling 
'completion-file-name-table':

   (completion-file-name-table "~user/" nil nil)
     => "~user/"

   (try-completion "~user/" '("~user/") nil)
     => t

So if we get 't' from 'try-completion', we "downgrade" that to the 
original string.

[1] 
https://www.gnu.org/software/emacs/manual/html_node/elisp/Basic-Completion.html

Subject: 30.0.50; [PATCH] Support completion of quoted variable refs in Eshell
From: Jim Porter <[email-masked]>
To: [email-masked]
Date: Wed, 1 Feb 2023 18:28:08 -0800
List-Id: <debbugs-submit.debbugs.gnu.org>

X-Debbugs-Cc: [email-masked]

Eshell lets you put quotes around variable names so that the parser can 
tell where the name ends, sort of like ${var} in other shells:

   ~ $ echo $'user-login-name'-suffix
   user-suffix

   ~ $ echo $"user-login-name"-suffix
   user-suffix

However, you can't tab-complete variable names when you do this. Here's 
a fix. I also fixed a couple small issues with completing directory 
names where it would sometimes complete to "whatever/ ". That extra 
trailing space isn't helpful, since you'd have to delete it before 
typing in a subdir.

Probably the most controversial part of this patch is in #0002, where I 
added another dynamic variable 'pcomplete-exit-function' that Pcomplete 
handlers can set to tell Pcomplete what to do after exiting a 
completion. Maybe it would be better to have handlers throw some special 
value for 'pcomplete-completions' that contains this info (sort of like 
the value that a 'completion-at-point-function' returns). I'm not sure 
what the best (and most-compatible) way to do this would be...

From 6f52adf5c1a183bc7e9d79438462df682855bfd6 Mon Sep 17 00:00:00 2001
From: Jim Porter <[email-masked]>
Date: Wed, 1 Feb 2023 17:48:37 -0800
Subject: [PATCH 1/3] ; Throw strings as the values for 'eshell-incomplete'

This lets us distinguish between cases like "'foo" and "$'foo".

* lisp/eshell/em-cmpl.el (eshell-complete-parse-arguments): Use
strings when checking the delimiter.

* lisp/eshell/em-glob.el (eshell-parse-glob-chars):
* lisp/eshell/em-pred.el (eshell-parse-arg-modifier):
* lisp/eshell/esh-arg.el (eshell-parse-backslash)
(eshell-parse-literal-quote, eshell-parse-double-quote)
(eshell-parse-special-reference):
* lisp/eshell/esh-cmd.el (eshell-parse-subcommand-argument)
(eshell-parse-lisp-argument):
* lisp/eshell/esh-var (eshell-parse-variable-ref)
(eshell-parse-indices): Throw strings instead of characters.

* lisp/eshell/esh-mode.el (eshell-parse-command-input): Print
delimiter as a string.
---
 lisp/eshell/em-cmpl.el  |  4 ++--
 lisp/eshell/em-glob.el  |  2 +-
 lisp/eshell/em-pred.el  |  2 +-
 lisp/eshell/esh-arg.el  |  8 ++++----
 lisp/eshell/esh-cmd.el  |  4 ++--
 lisp/eshell/esh-mode.el |  2 +-
 lisp/eshell/esh-var.el  | 18 +++++++++++-------
 7 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/lisp/eshell/em-cmpl.el b/lisp/eshell/em-cmpl.el
index acbf206a3c6..7f73922f370 100644
--- a/lisp/eshell/em-cmpl.el
+++ b/lisp/eshell/em-cmpl.el
@@ -332,10 +332,10 @@ eshell-complete-parse-arguments
 	      (catch 'eshell-incomplete
 		(ignore
 		 (setq args (eshell-parse-arguments begin end)))))
-	(cond ((memq (car delim) '(?\{ ?\<))
+        (cond ((member (car delim) '("{" "${" "$<"))
 	       (setq begin (1+ (cadr delim))
 		     args (eshell-parse-arguments begin end)))
-	      ((eq (car delim) ?\()
+              ((member (car delim) '("(" "$("))
 	       (throw 'pcompleted (elisp-completion-at-point)))
 	      (t
 	       (eshell--pcomplete-insert-tab))))
diff --git a/lisp/eshell/em-glob.el b/lisp/eshell/em-glob.el
index 716f5c32b87..1ac288ea226 100644
--- a/lisp/eshell/em-glob.el
+++ b/lisp/eshell/em-glob.el
@@ -170,7 +170,7 @@ eshell-parse-glob-chars
 	       (end (eshell-find-delimiter
 		     delim (if (eq delim ?\[) ?\] ?\)))))
 	  (if (not end)
-	      (throw 'eshell-incomplete delim)
+              (throw 'eshell-incomplete (char-to-string delim))
 	    (if (and (eshell-using-module 'eshell-pred)
 		     (eshell-arg-delimiter (1+ end)))
 		(ignore (goto-char here))
diff --git a/lisp/eshell/em-pred.el b/lisp/eshell/em-pred.el
index 14fa27aba06..2ccca092b86 100644
--- a/lisp/eshell/em-pred.el
+++ b/lisp/eshell/em-pred.el
@@ -293,7 +293,7 @@ eshell-parse-arg-modifier
     (forward-char)
     (let ((end (eshell-find-delimiter ?\( ?\))))
       (if (not end)
-	  (throw 'eshell-incomplete ?\()
+          (throw 'eshell-incomplete "(")
 	(when (eshell-arg-delimiter (1+ end))
 	  (save-restriction
 	    (narrow-to-region (point) end)
diff --git a/lisp/eshell/esh-arg.el b/lisp/eshell/esh-arg.el
index 6c882471aee..cb0b2e0938c 100644
--- a/lisp/eshell/esh-arg.el
+++ b/lisp/eshell/esh-arg.el
@@ -421,7 +421,7 @@ eshell-parse-backslash
 after are both returned."
   (when (eq (char-after) ?\\)
     (when (eshell-looking-at-backslash-return (point))
-	(throw 'eshell-incomplete ?\\))
+        (throw 'eshell-incomplete "\\"))
     (forward-char 2) ; Move one char past the backslash.
     (let ((special-chars (if eshell-current-quoted
                              eshell-special-chars-inside-quoting
@@ -447,7 +447,7 @@ eshell-parse-literal-quote
   (if (eq (char-after) ?\')
       (let ((end (eshell-find-delimiter ?\' ?\')))
 	(if (not end)
-	    (throw 'eshell-incomplete ?\')
+            (throw 'eshell-incomplete "'")
 	  (let ((string (buffer-substring-no-properties (1+ (point)) end)))
 	    (goto-char (1+ end))
 	    (while (string-match "''" string)
@@ -460,7 +460,7 @@ eshell-parse-double-quote
     (let* ((end (eshell-find-delimiter ?\" ?\" nil nil t))
 	   (eshell-current-quoted t))
       (if (not end)
-	  (throw 'eshell-incomplete ?\")
+          (throw 'eshell-incomplete "\"")
 	(prog1
 	    (save-restriction
 	      (forward-char)
@@ -514,7 +514,7 @@ eshell-parse-special-reference
                         t)) ;; buffer-p is non-nil by default.
             (end (eshell-find-delimiter ?\< ?\>)))
         (when (not end)
-          (throw 'eshell-incomplete ?\<))
+          (throw 'eshell-incomplete "#<"))
         (if (eshell-arg-delimiter (1+ end))
             (prog1
                 (list (if buffer-p 'get-buffer-create 'get-process)
diff --git a/lisp/eshell/esh-cmd.el b/lisp/eshell/esh-cmd.el
index b5f1d60ff18..972a6eda868 100644
--- a/lisp/eshell/esh-cmd.el
+++ b/lisp/eshell/esh-cmd.el
@@ -681,7 +681,7 @@ eshell-parse-subcommand-argument
 	       (not (eq (char-after (1+ (point))) ?\}))))
       (let ((end (eshell-find-delimiter ?\{ ?\})))
 	(if (not end)
-	    (throw 'eshell-incomplete ?\{)
+            (throw 'eshell-incomplete "{")
 	  (when (eshell-arg-delimiter (1+ end))
 	    (prog1
 		`(eshell-as-subcommand
@@ -698,7 +698,7 @@ eshell-parse-lisp-argument
 	      (condition-case nil
 		  (read (current-buffer))
 		(end-of-file
-		 (throw 'eshell-incomplete ?\()))))
+                 (throw 'eshell-incomplete "(")))))
 	(if (eshell-arg-delimiter)
 	    `(eshell-command-to-value
               (eshell-lisp-command (quote ,obj)))
diff --git a/lisp/eshell/esh-mode.el b/lisp/eshell/esh-mode.el
index 503d9ba1b63..d3e9823622f 100644
--- a/lisp/eshell/esh-mode.el
+++ b/lisp/eshell/esh-mode.el
@@ -567,7 +567,7 @@ eshell-parse-command-input
 		 (setq command (eshell-parse-command (cons beg end)
 						     args t)))))
 	(ignore
-	 (message "Expecting completion of delimiter %c ..."
+         (message "Expecting completion of delimiter %s ..."
 		  (if (listp delim)
 		      (car delim)
 		    delim)))
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index 60aab92b33e..a5bfbf4254d 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -503,7 +503,7 @@ eshell-parse-variable-ref
    ((eq (char-after) ?{)
     (let ((end (eshell-find-delimiter ?\{ ?\})))
       (if (not end)
-          (throw 'eshell-incomplete ?\{)
+          (throw 'eshell-incomplete "${")
         (forward-char)
         (prog1
             `(eshell-apply-indices
@@ -527,7 +527,7 @@ eshell-parse-variable-ref
    ((eq (char-after) ?\<)
     (let ((end (eshell-find-delimiter ?\< ?\>)))
       (if (not end)
-          (throw 'eshell-incomplete ?\<)
+          (throw 'eshell-incomplete "$<")
         (let* ((temp (make-temp-file temporary-file-directory))
                (cmd (concat (buffer-substring (1+ (point)) end)
                             " > " temp)))
@@ -560,15 +560,19 @@ eshell-parse-variable-ref
                         (current-buffer)))))
           indices ,eshell-current-quoted)
       (end-of-file
-       (throw 'eshell-incomplete ?\())))
+       (throw 'eshell-incomplete "$("))))
    ((looking-at (rx-to-string
                  `(or "'" ,(if eshell-current-quoted "\\\"" "\""))))
     (eshell-with-temp-command
         (or (eshell-unescape-inner-double-quote (point-max))
             (cons (point) (point-max)))
-      (let ((name (if (eq (char-after) ?\')
-                      (eshell-parse-literal-quote)
-                    (eshell-parse-double-quote))))
+      (let (name)
+        (when-let ((delim
+                    (catch 'eshell-incomplete
+                      (ignore (setq name (if (eq (char-after) ?\')
+                                             (eshell-parse-literal-quote)
+                                           (eshell-parse-double-quote)))))))
+          (throw 'eshell-incomplete (concat "$" delim)))
         (when name
           `(eshell-get-variable ,(eval name) indices ,eshell-current-quoted)))))
    ((assoc (char-to-string (char-after))
@@ -597,7 +601,7 @@ eshell-parse-indices
     (while (eq (char-after) ?\[)
       (let ((end (eshell-find-delimiter ?\[ ?\])))
 	(if (not end)
-	    (throw 'eshell-incomplete ?\[)
+            (throw 'eshell-incomplete "[")
 	  (forward-char)
           (eshell-with-temp-command (or (eshell-unescape-inner-double-quote end)
                                         (cons (point) end))
-- 
2.25.1

From fee5a8163d649c55f561f63582a64b99917aa8d2 Mon Sep 17 00:00:00 2001
From: Jim Porter <[email-masked]>
Date: Wed, 1 Feb 2023 17:48:43 -0800
Subject: [PATCH 2/3] Add support for completing quoted variables in Eshell
 like $'FOO'

This also adds the ability for Pcomplete handlers to set their own
exit functions that will get called as appropriate.

* lisp/pcomplete.el (pcomplete-completions-at-point): Check for
'pcomplete-exit-function' and call it if present.

* lisp/eshell/em-cmpl.el (eshell-complete-parse-arguments): Handle
quoted variables.  We also build the 'posns' list from right-to-left
now.

* lisp/eshell/esh-var.el (eshell-envvar-names): Ensure that variable
aliases are included in this list.
(eshell-complete-variable-reference): Handle quoted variables.
(eshell-variables-list): Handle quoted variables and set the exit
function on the completions.
(eshell-complete-variable-ref--exit): New function, extracted from
'eshell-variables-list'.

* test/lisp/eshell/em-cmpl-tests.el
(em-cmpl-test/quoted-variable-ref-completion)
(em-cmpl-test/variable-ref-completion/directory): New tests.
(em-cmpl-test/user-ref-completion): Fix typo.
---
 lisp/eshell/em-cmpl.el            | 19 +++++-----
 lisp/eshell/esh-var.el            | 60 +++++++++++++++++++------------
 lisp/pcomplete.el                 | 29 +++++++++------
 test/lisp/eshell/em-cmpl-tests.el | 25 +++++++++++++
 4 files changed, 92 insertions(+), 41 deletions(-)

diff --git a/lisp/eshell/em-cmpl.el b/lisp/eshell/em-cmpl.el
index 7f73922f370..380ecd0b91d 100644
--- a/lisp/eshell/em-cmpl.el
+++ b/lisp/eshell/em-cmpl.el
@@ -319,8 +319,7 @@ eshell-complete-parse-arguments
     (eshell--pcomplete-insert-tab))
   (let ((end (point-marker))
 	(begin (save-excursion (beginning-of-line) (point)))
-	(posns (list t))
-	args delim)
+	args posns delim)
     (when (and pcomplete-allow-modifications
 	       (memq this-command '(pcomplete-expand
 			            pcomplete-expand-and-complete)))
@@ -335,18 +334,22 @@ eshell-complete-parse-arguments
         (cond ((member (car delim) '("{" "${" "$<"))
 	       (setq begin (1+ (cadr delim))
 		     args (eshell-parse-arguments begin end)))
+              ((member (car delim) '("$'" "$\""))
+               ;; Add the (incomplete) argument to our arguments, and
+               ;; note its position.
+               (setq args (append (nth 2 delim) (list (car delim))))
+               (push (- (nth 1 delim) 2) posns))
               ((member (car delim) '("(" "$("))
 	       (throw 'pcompleted (elisp-completion-at-point)))
 	      (t
 	       (eshell--pcomplete-insert-tab))))
     (when (get-text-property (1- end) 'comment)
       (eshell--pcomplete-insert-tab))
-    (let ((pos begin))
-      (while (< pos end)
-	(if (get-text-property pos 'arg-begin)
-	    (nconc posns (list pos)))
-	(setq pos (1+ pos))))
-    (setq posns (cdr posns))
+    (let ((pos (1- end)))
+      (while (>= pos begin)
+        (when (get-text-property pos 'arg-begin)
+          (push pos posns))
+        (setq pos (1- pos))))
     (cl-assert (= (length args) (length posns)))
     (let ((a args) (i 0) new-start)
       (while a
diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
index a5bfbf4254d..952559e0d18 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -434,9 +434,15 @@ eshell-insert-envvar
 
 (defun eshell-envvar-names (&optional environment)
   "Return a list of currently visible environment variable names."
-  (mapcar (lambda (x)
-            (substring x 0 (string-search "=" x)))
-	  (or environment process-environment)))
+  (delete-dups
+   (append
+    ;; Real environment variables
+    (mapcar (lambda (x)
+              (substring x 0 (string-search "=" x)))
+	    (or environment process-environment))
+    ;; Eshell variable aliases
+    (mapcar (lambda (x) (car x))
+            eshell-variable-aliases-list))))
 
 (defun eshell-environment-variables ()
   "Return a `process-environment', fully updated.
@@ -820,33 +826,41 @@ eshell-complete-variable-reference
   (let ((arg (pcomplete-actual-arg)))
     (when (string-match
            (rx "$" (? (or "#" "@"))
-               (? (group (regexp eshell-variable-name-regexp)))
-               string-end)
+               (? (or (group-n 1 (regexp eshell-variable-name-regexp)
+                               string-end)
+                      (seq (group-n 2 (or "'" "\""))
+                           (group-n 1 (+ anychar))))))
            arg)
       (setq pcomplete-stub (substring arg (match-beginning 1)))
+      (setq pcomplete-exit-function
+            (apply-partially #'eshell-complete-variable-ref--exit
+                             (match-string 2 arg)))
       (throw 'pcomplete-completions (eshell-variables-list)))))
 
 (defun eshell-variables-list ()
   "Generate list of applicable variables."
-  (let ((argname pcomplete-stub)
-	completions)
-    (dolist (alias eshell-variable-aliases-list)
-      (if (string-match (concat "^" argname) (car alias))
-	  (setq completions (cons (car alias) completions))))
+  (let ((argname pcomplete-stub))
     (sort
-     (append
-      (mapcar
-       (lambda (varname)
-         (let ((value (eshell-get-variable varname)))
-           (if (and value
-                    (stringp value)
-                    (file-directory-p value))
-               (concat varname "/")
-             varname)))
-       (eshell-envvar-names (eshell-environment-variables)))
-      (all-completions argname obarray 'boundp)
-      completions)
-     'string-lessp)))
+     (append (eshell-envvar-names)
+             (all-completions argname obarray 'boundp))
+     #'string-lessp)))
+
+(defun eshell-complete-variable-ref--exit (delimiter variable status)
+  "An exit function for completing Eshell variable references.
+DELIMITER is a delimiter wrapping the variable (' or \") or nil.
+VARIABLE is the name of the variable that was just completed.
+STATUS is a symbol representing the state of the completion."
+  (when (eq status 'finished)
+    (when delimiter
+      (if (looking-at (regexp-quote delimiter))
+          (goto-char (match-end 0))
+        (insert delimiter)))
+    (let ((non-essential t)
+          (value (eshell-get-variable variable)))
+      (when (and (stringp value) (file-directory-p value))
+        (insert "/")
+        ;; Tell Pcomplete not to insert its own termination string.
+        t))))
 
 (defun eshell-complete-variable-assignment ()
   "If there is a variable assignment, allow completion of entries."
diff --git a/lisp/pcomplete.el b/lisp/pcomplete.el
index 1ca7a213361..ad50493ac28 100644
--- a/lisp/pcomplete.el
+++ b/lisp/pcomplete.el
@@ -357,6 +357,7 @@ pcomplete-begins
 (defvar pcomplete-last nil)
 (defvar pcomplete-index nil)
 (defvar pcomplete-stub nil)
+(defvar pcomplete-exit-function nil)
 (defvar pcomplete-seen nil)
 (defvar pcomplete-norm-func nil)
 
@@ -406,6 +407,7 @@ pcomplete-completions-at-point
             (if pcomplete-allow-modifications buffer-read-only t))
            pcomplete-seen pcomplete-norm-func
            pcomplete-args pcomplete-last pcomplete-index
+           pcomplete-exit-function
            (pcomplete-autolist pcomplete-autolist)
            (pcomplete-suffix-list pcomplete-suffix-list)
            ;; Apparently the vars above are global vars modified by
@@ -494,16 +496,23 @@ pcomplete-completions-at-point
                     (get-text-property 0 'pcomplete-help cand)))
                 :predicate pred
                 :exit-function
-		;; If completion is finished, add a terminating space.
-		;; We used to also do this if STATUS is `sole', but
-		;; that does not work right when completion cycling.
-                (unless (zerop (length pcomplete-termination-string))
-                  (lambda (_s status)
-                    (when (eq status 'finished)
-                      (if (looking-at
-                           (regexp-quote pcomplete-termination-string))
-                          (goto-char (match-end 0))
-                        (insert pcomplete-termination-string)))))))))))
+                (when (or pcomplete-exit-function
+                          (length> pcomplete-termination-string 0))
+                  (let ((exit-function pcomplete-exit-function))
+                    (lambda (string status)
+                      (let ((terminate-p t))
+                        (when exit-function
+                          (setq terminate-p (not (funcall exit-function
+                                                          string status))))
+		        ;; If completion is finished, add a terminating
+		        ;; space.  We used to also do this if STATUS is
+		        ;; `sole', but that does not work right when
+		        ;; completion cycling.
+                        (when (and terminate-p (eq status 'finished))
+                          (if (looking-at
+                               (regexp-quote pcomplete-termination-string))
+                              (goto-char (match-end 0))
+                            (insert pcomplete-termination-string)))))))))))))
 
  ;; I don't think such commands are usable before first setting up buffer-local
  ;; variables to parse args, so there's no point autoloading it.
diff --git a/test/lisp/eshell/em-cmpl-tests.el b/test/lisp/eshell/em-cmpl-tests.el
index 12a156fbb38..1f8c571c44c 100644
--- a/test/lisp/eshell/em-cmpl-tests.el
+++ b/test/lisp/eshell/em-cmpl-tests.el
@@ -183,6 +183,31 @@ em-cmpl-test/variable-ref-completion
    (should (equal (eshell-insert-and-complete "echo $system-nam")
                   "echo $system-name "))))
 
+(ert-deftest em-cmpl-test/quoted-variable-ref-completion ()
+  "Test completion of variable references like \"$'var'\".
+See <lisp/eshell/esh-var.el>."
+  (with-temp-eshell
+   (should (equal (eshell-insert-and-complete "echo $'system-nam")
+                  "echo $'system-name' ")))
+  (with-temp-eshell
+   (should (equal (eshell-insert-and-complete "echo $\"system-nam")
+                  "echo $\"system-name\" "))))
+
+(ert-deftest em-cmpl-test/variable-ref-completion/directory ()
+  "Test completion of variable references that expand to directories.
+See <lisp/eshell/esh-var.el>."
+  (with-temp-eshell
+   (should (equal (eshell-insert-and-complete "echo $PW")
+                  "echo $PWD/")))
+  (with-temp-eshell
+   (let ((minibuffer-message-timeout 0)
+         (inhibit-message t))
+     (should (equal (eshell-insert-and-complete "echo $PWD")
+                    "echo $PWD/"))))
+  (with-temp-eshell
+   (should (equal (eshell-insert-and-complete "echo $'PW")
+                  "echo $'PWD'/"))))
+
 (ert-deftest em-cmpl-test/variable-assign-completion ()
   "Test completion of variable assignments like \"var=value\".
 See <lisp/eshell/esh-var.el>."
-- 
2.25.1

From ad7dbf7e455dc6ed926b51a4b936a34d52eb3500 Mon Sep 17 00:00:00 2001
From: Jim Porter <[email-masked]>
Date: Wed, 1 Feb 2023 17:48:47 -0800
Subject: [PATCH 3/3] Don't add a space after the trailing slash when
 completing ~FOO in Eshell

* lisp/eshell/em-dirs.el (eshell-complete-user-reference): Set exit
function.
(eshell-complete-user-ref--exit): New function.

* test/lisp/eshell/em-cmpl-tests.el
(em-cmpl-test/user-ref-completion): Update test.
---
 lisp/eshell/em-dirs.el            | 16 +++++++++++-----
 test/lisp/eshell/em-cmpl-tests.el |  5 ++---
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/lisp/eshell/em-dirs.el b/lisp/eshell/em-dirs.el
index 0d02b64b084..c6e384be374 100644
--- a/lisp/eshell/em-dirs.el
+++ b/lisp/eshell/em-dirs.el
@@ -281,15 +281,21 @@ eshell-complete-user-reference
   (let ((arg (pcomplete-actual-arg)))
     (when (string-match "\\`~[a-z]*\\'" arg)
       (setq pcomplete-stub (substring arg 1)
-	    pcomplete-last-completion-raw t)
+            pcomplete-last-completion-raw t
+            pcomplete-exit-function #'eshell-complete-user-ref--exit)
       (throw 'pcomplete-completions
 	     (progn
 	       (eshell-read-user-names)
 	       (pcomplete-uniquify-list
-		(mapcar
-                 (lambda (user)
-                   (file-name-as-directory (cdr user)))
-		 eshell-user-names)))))))
+                (mapcar #'cdr eshell-user-names)))))))
+
+(defun eshell-complete-user-ref--exit (_ status)
+  "An exit function for completing Eshell user references.
+STATUS is a symbol representing the state of the completion."
+  (when (eq status 'finished)
+    (insert "/")
+    ;; Tell Pcomplete not to insert its own termination string.
+    t))
 
 (defun eshell/pwd (&rest _args)
   "Change output from `pwd' to be cleaner."
diff --git a/test/lisp/eshell/em-cmpl-tests.el b/test/lisp/eshell/em-cmpl-tests.el
index 1f8c571c44c..ecab7332822 100644
--- a/test/lisp/eshell/em-cmpl-tests.el
+++ b/test/lisp/eshell/em-cmpl-tests.el
@@ -218,15 +218,14 @@ em-cmpl-test/variable-assign-completion
                     "VAR=file.txt ")))))
 
 (ert-deftest em-cmpl-test/user-ref-completion ()
-  "Test completeion of user references like \"~user\".
+  "Test completion of user references like \"~user\".
 See <lisp/eshell/em-dirs.el>."
   (unwind-protect
       (with-temp-eshell
        (cl-letf (((symbol-function 'eshell-read-user-names)
                   (lambda () (setq eshell-user-names '((1234 . "user"))))))
-         ;; FIXME: Should this really add a space at the end?
          (should (equal (eshell-insert-and-complete "echo ~us")
-                        "echo ~user/ "))))
+                        "echo ~user/"))))
     ;; Clear the cached user names we set above.
     (setq eshell-user-names nil)))
 
-- 
2.25.1