From: Stefan Monnier <monnier@iro.umontreal.ca>
To: Nicolas Martyanoff <nicolas@n16f.net>
Cc: emacs-devel@gnu.org
Subject: Re: [PATCH] em-cmpl: fix completion of command paths
Date: Wed, 25 Jan 2023 12:03:38 -0500 [thread overview]
Message-ID: <jwvtu0ek0ps.fsf-monnier+emacs-devel@gnu.org> (raw)
In-Reply-To: <20230107121943.44658-1-nicolas@n16f.net> (Nicolas Martyanoff's message of "Sat, 7 Jan 2023 13:19:44 +0100")
Hi Nicolas,
Terribly sorry for the long delay to answer.
> The use of `completion-table-dynamic' was introduced in 899055e to fix
> bug#48995. However the following issue remains: when completing a command
> path, absolute ("/usr/bin/") or relative ("./subdir/"), a space is
> automatically added at the end.
> Bypassing `completion-table-dynamic' for filenames containing a directory
> part fixes the final space bug and does not reintroduce bug#48995.
This looks very good.
IIUC this fixes a regression w.r.t Emacs-28, so it would be nice to
install it into `emacs-29`.
Eli, what do you think?
Stefan
> As a result, `eshell--pcomplete-executables' is not needed anymore, it was
> only necessary to work around the way `completion-table-dynamic' works.
>
> Thanks to Stefan Monnier for his input on the problem.
> ---
> lisp/eshell/em-cmpl.el | 127 ++++++++++++++++++-----------------------
> 1 file changed, 56 insertions(+), 71 deletions(-)
>
> diff --git a/lisp/eshell/em-cmpl.el b/lisp/eshell/em-cmpl.el
> index ca51cee2558..c5427fa1a6d 100644
> --- a/lisp/eshell/em-cmpl.el
> +++ b/lisp/eshell/em-cmpl.el
> @@ -378,31 +378,6 @@
> args)
> posns)))
>
> -(defun eshell--pcomplete-executables ()
> - "Complete amongst a list of directories and executables.
> -
> -Wrapper for `pcomplete-executables' or `pcomplete-dirs-or-entries',
> -depending on the value of `eshell-force-execution'.
> -
> -Adds path prefix to candidates independent of `action' value."
> - ;; `pcomplete-entries' returns filenames without path on `action' to
> - ;; use current string directory as done in `completion-file-name-table'
> - ;; when `action' is nil to construct executable candidates.
> - (let ((table (if eshell-force-execution
> - (pcomplete-dirs-or-entries nil #'file-readable-p)
> - (pcomplete-executables))))
> - (lambda (string pred action)
> - (let ((cands (funcall table string pred action)))
> - (if (eq action t)
> - (let ((specdir (file-name-directory string)))
> - (mapcar
> - (lambda (cand)
> - (if (stringp cand)
> - (file-name-concat specdir cand)
> - cand))
> - cands))
> - cands)))))
> -
> (defun eshell--complete-commands-list ()
> "Generate list of applicable, visible commands."
> ;; Building the commands list can take quite a while, especially over Tramp
> @@ -413,11 +413,21 @@
> ;; we complete. Adjust `pcomplete-stub' accordingly!
> (if (and (> (length pcomplete-stub) 0)
> (eq (aref pcomplete-stub 0) eshell-explicit-command-char))
> - (setq pcomplete-stub (substring pcomplete-stub 1)))))
> + (setq pcomplete-stub (substring pcomplete-stub 1))))
> + (filename (pcomplete-arg)))
> + ;; Do not use `completion-table-dynamic' when completing a command path
> + ;; (absolute or relative): doing so assumes that the subpath in the input
> + ;; string is always a command, and appends a space character, which is
> + ;; incorrect (i.e. "/usr/bi" should yield "/usr/bin/" after completion,
> + ;; not "/usr/bin/ ").
> + ;;
> + ;; If you work on this function, be careful not to reintroduce bug#48995.
> + (if (file-name-directory filename)
> + (if eshell-force-execution
> + (pcomplete-dirs-or-entries nil #'file-readable-p)
> + (pcomplete-executables))
> (completion-table-dynamic
> (lambda (filename)
> - (if (file-name-directory filename)
> - (eshell--pcomplete-executables)
> (let* ((paths (eshell-get-path))
> (cwd (file-name-as-directory
> (expand-file-name default-directory)))
next prev parent reply other threads:[~2023-01-25 17:03 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-07 12:19 [PATCH] em-cmpl: fix completion of command paths Nicolas Martyanoff
2023-01-07 12:23 ` Nicolas Martyanoff
2023-01-13 10:59 ` Nicolas Martyanoff
2023-01-25 17:03 ` Stefan Monnier [this message]
2023-01-25 17:15 ` Eli Zaretskii
2023-01-25 20:32 ` [PATCH] Update compat Nicolas Martyanoff
[not found] ` <87bkmmwcwx.fsf@valhala.localdomain>
2023-01-26 6:02 ` [PATCH] em-cmpl: fix completion of command paths Eli Zaretskii
2023-01-26 9:06 ` Eli Zaretskii
2023-01-26 12:21 ` Nicolas Martyanoff
2023-01-26 12:58 ` Eli Zaretskii
2023-01-28 0:11 ` Stefan Monnier
2023-01-28 8:27 ` Eli Zaretskii
2023-01-28 0:12 ` Stefan Monnier
2023-01-25 21:50 ` Jim Porter
2023-02-08 6:06 ` Jim Porter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=jwvtu0ek0ps.fsf-monnier+emacs-devel@gnu.org \
--to=monnier@iro.umontreal.ca \
--cc=emacs-devel@gnu.org \
--cc=nicolas@n16f.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.