* bug#61221: 30.0.50; [PATCH] Support completion of quoted variable refs in Eshell
@ 2023-02-02 2:28 Jim Porter
2023-02-05 15:33 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 1 reply; 7+ messages in thread
From: Jim Porter @ 2023-02-02 2:28 UTC (permalink / raw)
To: 61221; +Cc: monnier
[-- Attachment #1: Type: text/plain, Size: 1075 bytes --]
X-Debbugs-Cc: monnier@iro.umontreal.ca
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...
[-- Attachment #2: 0001-Throw-strings-as-the-values-for-eshell-incomplete.patch --]
[-- Type: text/plain, Size: 8246 bytes --]
From 6f52adf5c1a183bc7e9d79438462df682855bfd6 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
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
[-- Attachment #3: 0002-Add-support-for-completing-quoted-variables-in-Eshel.patch --]
[-- Type: text/plain, Size: 10781 bytes --]
From fee5a8163d649c55f561f63582a64b99917aa8d2 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
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
[-- Attachment #4: 0003-Don-t-add-a-space-after-the-trailing-slash-when-comp.patch --]
[-- Type: text/plain, Size: 2793 bytes --]
From ad7dbf7e455dc6ed926b51a4b936a34d52eb3500 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* bug#61221: 30.0.50; [PATCH] Support completion of quoted variable refs in Eshell
2023-02-02 2:28 bug#61221: 30.0.50; [PATCH] Support completion of quoted variable refs in Eshell Jim Porter
@ 2023-02-05 15:33 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-23 6:42 ` Jim Porter
0 siblings, 1 reply; 7+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-02-05 15:33 UTC (permalink / raw)
To: Jim Porter; +Cc: 61221
> 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...
BTW, a low-tech way to get similar results is to change your completion
table. Instead of having entries like
user-login-name
make it contains entries like:
user-login-name"
or
user-login-name'
or even
'user-login-name'
[ assuming you change the `pcomplete-stub` so that it also contains the
leading ' of course. ]
Regarding `pcomplete-exit-function`:
- It's ugly and I don't like it, but given the general design of the
current Pcomplete API, I think it's OK.
- Please add a good docstring to that variable (the fact that the other
vars nearby don't have any is no excuse, instead it's a(nother)
problem that should be fixed).
- Maybe `pcomplete-exit-function` should override the default exit
function (i.e. the handing of `pcomplete-termination-string`) rather
than being added to it?
E.g. we could start with `pcomplete-exit-function` bound to the
default (the one that handles `pcomplete-termination-string`) and let
the rest of the code modify it either with `setq` (to completely
override) or `add-function`.
Stefan
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#61221: 30.0.50; [PATCH] Support completion of quoted variable refs in Eshell
2023-02-05 15:33 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-02-23 6:42 ` Jim Porter
2023-02-23 18:02 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 1 reply; 7+ messages in thread
From: Jim Porter @ 2023-02-23 6:42 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 61221
[-- Attachment #1: Type: text/plain, Size: 1728 bytes --]
On 2/5/2023 7:33 AM, Stefan Monnier via Bug reports for GNU Emacs, the
Swiss army knife of text editors wrote:
> BTW, a low-tech way to get similar results is to change your completion
> table. Instead of having entries like
>
> user-login-name
>
> make it contains entries like:
>
> user-login-name" ...
I'd thought about this, but it gets a little uglier for more-complex
cases like bug#53371, which wants to complete things like the "foo" in
"#<buffer foo>".
> Regarding `pcomplete-exit-function`:
> - It's ugly and I don't like it, but given the general design of the
> current Pcomplete API, I think it's OK.
Yeah, I'd rather do it by returning some structured object, but that
would probably require some significant incompatible changes to pcomplete...
> - Please add a good docstring to that variable (the fact that the other
> vars nearby don't have any is no excuse, instead it's a(nother)
> problem that should be fixed).
Done.
> - Maybe `pcomplete-exit-function` should override the default exit
> function (i.e. the handing of `pcomplete-termination-string`) rather
> than being added to it?
> E.g. we could start with `pcomplete-exit-function` bound to the
> default (the one that handles `pcomplete-termination-string`) and let
> the rest of the code modify it either with `setq` (to completely
> override) or `add-function`.
I like this a lot better than my previous solution. Done.
I also changed the last patch to return a programmed completion function
that mimics "~user" completion in 'completion-file-name-table'. That
seems like a more-consistent way to handle that case, since now Eshell
"~user" completion works the same as completing any file name.
[-- Attachment #2: 0001-Throw-strings-as-the-values-for-eshell-incomplete.patch --]
[-- Type: text/plain, Size: 8246 bytes --]
From 187a9cfa2a8e7c78eb6bac338778325741ee0457 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
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 af8ac4278f1..5625c53dc9b 100644
--- a/lisp/eshell/em-cmpl.el
+++ b/lisp/eshell/em-cmpl.el
@@ -330,10 +330,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 c7360fb246e..8a2ba13b2ad 100644
--- a/lisp/eshell/em-glob.el
+++ b/lisp/eshell/em-glob.el
@@ -171,7 +171,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 efc46f10c96..d609711402a 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 b3cde472713..0c381dbb86a 100644
--- a/lisp/eshell/esh-mode.el
+++ b/lisp/eshell/esh-mode.el
@@ -580,7 +580,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
[-- Attachment #3: 0002-Add-support-for-completing-quoted-variables-in-Eshel.patch --]
[-- Type: text/plain, Size: 11013 bytes --]
From 24f8b94876e6cdc6369c0c747d2ce4febd3aa4ea Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
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-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 | 63 +++++++++++++++++++------------
lisp/pcomplete.el | 38 ++++++++++++++-----
test/lisp/eshell/em-cmpl-tests.el | 25 ++++++++++++
4 files changed, 102 insertions(+), 43 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 @@ 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)))
@@ -333,18 +332,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..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))
+ eshell-variable-aliases-list))))
(defun eshell-environment-variables ()
"Return a `process-environment', fully updated.
@@ -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))
+ ;; 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 @@ pcomplete-norm-func
;;; 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 @@ 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-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 @@ 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)))))))))))
+ 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 @@ 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
[-- Attachment #4: 0003-Don-t-add-a-space-after-the-trailing-slash-when-comp.patch --]
[-- Type: text/plain, Size: 3993 bytes --]
From c191af5dc286efd3d3b8a8d66ee280d17ef99853 Mon Sep 17 00:00:00 2001
From: Jim Porter <jporterbugs@gmail.com>
Date: Wed, 1 Feb 2023 17:48:47 -0800
Subject: [PATCH 3/3] Don't add a space after the trailing slash when
completing ~USER in Eshell
This provides a programmed completion function that works similarly to
~USER completion in 'completion-file-name-table'.
* lisp/eshell/em-dirs.el (eshell-complete-user-reference): Throw a
programmed completion function.
* test/lisp/eshell/em-cmpl-tests.el
(em-cmpl-test/user-ref-completion): Update test.
---
lisp/eshell/em-dirs.el | 37 +++++++++++++++++++++++--------
test/lisp/eshell/em-cmpl-tests.el | 5 ++---
2 files changed, 30 insertions(+), 12 deletions(-)
diff --git a/lisp/eshell/em-dirs.el b/lisp/eshell/em-dirs.el
index 0d02b64b084..62d37e8f9fe 100644
--- a/lisp/eshell/em-dirs.el
+++ b/lisp/eshell/em-dirs.el
@@ -281,15 +281,34 @@ 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)
- (throw 'pcomplete-completions
- (progn
- (eshell-read-user-names)
- (pcomplete-uniquify-list
- (mapcar
- (lambda (user)
- (file-name-as-directory (cdr user)))
- eshell-user-names)))))))
+ pcomplete-last-completion-raw t)
+ ;; pcomplete-exit-function #'eshell-complete-user-ref--exit)
+ (eshell-read-user-names)
+ (let ((names (pcomplete-uniquify-list
+ (mapcar (lambda (user)
+ (file-name-as-directory (cdr user)))
+ eshell-user-names))))
+ (throw 'pcomplete-completions
+ ;; Provide a programmed completion table. This works
+ ;; just like completing over the list of names, except
+ ;; it always returns the completed string, never `t'.
+ ;; That's because this is only completing a directory
+ ;; name, and so the completion isn't actually finished
+ ;; yet.
+ (lambda (string pred action)
+ (pcase action
+ ('nil ; try-completion
+ (let ((result (try-completion string names pred)))
+ (if (eq result t) string result)))
+ ('t ; all-completions
+ (all-completions string names pred))
+ ('lambda ; test-completion
+ (let ((result (test-completion string names pred)))
+ (if (eq result t) string result)))
+ ('metadata
+ '(metadata (category . file)))
+ (`(boundaries . ,suffix)
+ `(boundaries 0 . ,(string-search "/" suffix))))))))))
(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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* bug#61221: 30.0.50; [PATCH] Support completion of quoted variable refs in Eshell
2023-02-23 6:42 ` Jim Porter
@ 2023-02-23 18:02 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-23 22:11 ` Jim Porter
0 siblings, 1 reply; 7+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-02-23 18:02 UTC (permalink / raw)
To: Jim Porter; +Cc: 61221
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#61221: 30.0.50; [PATCH] Support completion of quoted variable refs in Eshell
2023-02-23 18:02 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-02-23 22:11 ` Jim Porter
2023-02-23 22:39 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 1 reply; 7+ messages in thread
From: Jim Porter @ 2023-02-23 22:11 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 61221-done
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#61221: 30.0.50; [PATCH] Support completion of quoted variable refs in Eshell
2023-02-23 22:11 ` Jim Porter
@ 2023-02-23 22:39 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-23 23:17 ` Jim Porter
0 siblings, 1 reply; 7+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-02-23 22:39 UTC (permalink / raw)
To: Jim Porter; +Cc: 61221-done
>>> + ('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).
What you describe is for `try-completion`, not `test-completion`.
> 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
The `action` arg is not `lambda` is these examples.
Stefan
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#61221: 30.0.50; [PATCH] Support completion of quoted variable refs in Eshell
2023-02-23 22:39 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-02-23 23:17 ` Jim Porter
0 siblings, 0 replies; 7+ messages in thread
From: Jim Porter @ 2023-02-23 23:17 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 61221-done
On 2/23/2023 2:39 PM, Stefan Monnier via Bug reports for GNU Emacs, the
Swiss army knife of text editors wrote:
>> 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).
>
> What you describe is for `try-completion`, not `test-completion`.
>
>> 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
>
> The `action` arg is not `lambda` is these examples.
Ohhh. I totally missed that part. Pushed a fix as 0a361fd91a. Thanks
again for finding my mistakes.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-02-23 23:17 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-02 2:28 bug#61221: 30.0.50; [PATCH] Support completion of quoted variable refs in Eshell Jim Porter
2023-02-05 15:33 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-23 6:42 ` Jim Porter
2023-02-23 18:02 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-23 22:11 ` Jim Porter
2023-02-23 22:39 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-23 23:17 ` Jim Porter
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).