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

[Stefan Monnier]

The patches look good, feel free to push, AFAIC.
Some minor comments below.


        Stefan


> diff --git a/lisp/eshell/esh-var.el b/lisp/eshell/esh-var.el
> index a5bfbf4254d..e8e8cfb39b4 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))

Aka (mapcar #'car

> @@ -817,36 +823,43 @@ eshell-index-value
>  
>  (defun eshell-complete-variable-reference ()
>    "If there is a variable reference, complete it."
> -  (let ((arg (pcomplete-actual-arg)))
> +  (let ((arg (pcomplete-actual-arg))
> +        delimiter)
>      (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-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).

> +     (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 :-)

> +                   ('lambda               ; test-completion
> +                     (let ((result (test-completion string names pred)))
> +                       (if (eq result t) string result)))

Hmm... why not just always return `result`?


        Stefan