master 4b364a990a0 2/3: Add support for completing quoted variables in Eshell like $'FOO'

[Jim Porter]

branch: master
commit 4b364a990a06cd73b553fdc9e9a65bc7398dbea6
Author: Jim Porter <jporterbugs@gmail.com>
Commit: Jim Porter <jporterbugs@gmail.com>

    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-default-exit-function): New function.
    (pcomplete-exit-function): New variable...
    (pcomplete-completions-at-point): ... let-bind and use it.
    
    * 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 and set
    the exit function on the completions.
    (eshell-variables-list): Simplify.  We now add the trailing slash for
    directories in the exit function inside
    'eshell-complete-variable-reference'.
    
    * test/lisp/eshell/em-cmpl-tests.el
    (em-cmpl-test/quoted-variable-ref-completion)
    (em-cmpl-test/variable-ref-completion/directory): New tests.
---
 lisp/eshell/em-cmpl.el            | 19 +++++++------
 lisp/eshell/esh-var.el            | 58 +++++++++++++++++++++++----------------
 lisp/pcomplete.el                 | 38 ++++++++++++++++++-------
 test/lisp/eshell/em-cmpl-tests.el | 25 +++++++++++++++++
 4 files changed, 99 insertions(+), 41 deletions(-)

diff --git a/lisp/eshell/em-cmpl.el b/lisp/eshell/em-cmpl.el
index 5625c53dc9b..5dfd10d6e4c 100644
--- a/lisp/eshell/em-cmpl.el
+++ b/lisp/eshell/em-cmpl.el
@@ -317,8 +317,7 @@ to writing a completion function."
     (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)))
@@ -333,18 +332,22 @@ to writing a completion function."
         (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..0031324b537 100644
--- a/lisp/eshell/esh-var.el
+++ b/lisp/eshell/esh-var.el
@@ -434,9 +434,14 @@ the values of nil for each."
 
 (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 #'car eshell-variable-aliases-list))))
 
 (defun eshell-environment-variables ()
   "Return a `process-environment', fully updated.
@@ -820,33 +825,40 @@ START and END."
   (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)))
+      (let ((delimiter (match-string 2 arg)))
+        ;; When finished with completion, insert the trailing
+        ;; delimiter, if any, and add a trailing slash if the variable
+        ;; refers to a directory.
+        (add-function
+         :before-until (var pcomplete-exit-function)
+         (lambda (variable status)
+           (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))))))
       (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-assignment ()
   "If there is a variable assignment, allow completion of entries."
diff --git a/lisp/pcomplete.el b/lisp/pcomplete.el
index 1ca7a213361..36f68f1af57 100644
--- a/lisp/pcomplete.el
+++ b/lisp/pcomplete.el
@@ -362,6 +362,32 @@ modified to be an empty string, or the desired separation 
string."
 
 ;;; User Functions:
 
+(defun pcomplete-default-exit-function (_s status)
+  "The default exit function to use in `pcomplete-completions-at-point'.
+This just adds `pcomplete-termination-string' after the
+completion if STATUS is `finished'."
+  (unless (zerop (length pcomplete-termination-string))
+    (when (eq status 'finished)
+      (if (looking-at
+           (regexp-quote pcomplete-termination-string))
+          (goto-char (match-end 0))
+        (insert pcomplete-termination-string)))))
+
+(defvar pcomplete-exit-function #'pcomplete-default-exit-function
+  "The exit function to call in `pcomplete-completions-at-point'.
+
+This variable is let-bound in `pcomplete-completions-at-point',
+so you can modify or advise it in order to adjust the behavior
+for a specific completion.  For example, you might do the
+following in a `pcomplete-try-first-hook' function to insert a
+trailing slash after a completion:
+
+  (add-function
+   :before (var pcomplete-exit-function)
+   (lambda (_ status)
+     (when (eq status \\='finished)
+       (insert \"/\"))))")
+
 ;;; Alternative front-end using the standard completion facilities.
 
 ;; The way pcomplete-parse-arguments and pcomplete-stub work only
@@ -406,6 +432,7 @@ Same as `pcomplete' but using the standard completion UI."
             (if pcomplete-allow-modifications buffer-read-only t))
            pcomplete-seen pcomplete-norm-func
            pcomplete-args pcomplete-last pcomplete-index
+           (pcomplete-exit-function pcomplete-exit-function)
            (pcomplete-autolist pcomplete-autolist)
            (pcomplete-suffix-list pcomplete-suffix-list)
            ;; Apparently the vars above are global vars modified by
@@ -494,16 +521,7 @@ Same as `pcomplete' but using the standard completion UI."
                     (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)))))))))))
+                pcomplete-exit-function))))))
 
  ;; 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 @@ See <lisp/eshell/esh-var.el>."
    (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>."