* bug#57905: [PATCH] Fix eshell directory and executable completion on action t
@ 2022-09-17 22:55 Daniel Pettersson
2022-09-18 7:40 ` Eli Zaretskii
2022-09-18 10:41 ` Lars Ingebrigtsen
0 siblings, 2 replies; 7+ messages in thread
From: Daniel Pettersson @ 2022-09-17 22:55 UTC (permalink / raw
To: 57905
When eshell directory/executable completion results in multiple candidates
the candidates are striped of their file path.
emacs -Q
M-x eshell RET
touch test1 test2
chmod +x test1 test2
./test<Tab>
In *Completions* buffer:
2 possible completions:
test1
test2
Expected:
./test1
./test2
Same issue for directories:
mkdir test_dir1 test_dir2
./test_dir<Tab>
In *Completions* buffer:
2 possible completions:
test1_dir1
test2_dir2
This also the case when the candidates are not in the working directory:
mkdir -p 1/2/3/4
touch 1/2/3/4/test1 1/2/3/4/test2
chmod +x 1/2/3/4/test1 1/2/3/4/test2
./1/2/3/4/test<Tab>
In *Completions* buffer:
2 possible completions:
test1
test2
This issue is not present with one completion as further down the call
stack `completion-file-name-table' is called with action t, which concates
completion string directory with completion candidate.
Possible solution:
---
lisp/eshell/em-cmpl.el | 29 ++++++++++++++++++++++++++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/lisp/eshell/em-cmpl.el b/lisp/eshell/em-cmpl.el
index 822cc94149..d9261fae27 100644
--- a/lisp/eshell/em-cmpl.el
+++ b/lisp/eshell/em-cmpl.el
@@ -378,6 +378,31 @@ eshell-complete-parse-arguments
args)
posns)))
+(defun eshell--pcomplete-executables ()
+ "Complete amongs 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' t
+ ;; 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)
+ (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
@@ -392,9 +417,7 @@ eshell--complete-commands-list
(completion-table-dynamic
(lambda (filename)
(if (file-name-directory filename)
- (if eshell-force-execution
- (pcomplete-dirs-or-entries nil #'file-readable-p)
- (pcomplete-executables))
+ (eshell--pcomplete-executables)
(let* ((paths (eshell-get-path))
(cwd (file-name-as-directory
(expand-file-name default-directory)))
--
2.30.1 (Apple Git-130)
Issue on 28.2.0 as well.
In GNU Emacs 29.0.50 (build 1, aarch64-apple-darwin20.6.0, NS
appkit-2022.60 Version 11.5.2 (Build 20G95)) of 2022-09-16 built on
Daniels-Air
Repository revision: fe7c015b20b5bca07aa178d28b9fd5cc66ad16f9
Repository branch: master
Windowing system distributor 'Apple', version 10.3.2022
System Description: macOS 11.5.2
Configured features:
ACL DBUS GLIB GNUTLS JSON LCMS2 LIBXML2 MODULES NOTIFY KQUEUE NS PDUMPER
PNG RSVG SQLITE3 THREADS TOOLKIT_SCROLL_BARS WEBP XIM ZLIB
Important settings:
value of $LC_CTYPE: UTF-8
locale-coding-system: utf-8-unix
Major mode: Eshell
Minor modes in effect:
shell-dirtrack-mode: t
eshell-prompt-mode: t
eshell-hist-mode: t
eshell-pred-mode: t
eshell-cmpl-mode: t
eshell-proc-mode: t
eshell-arg-mode: t
tooltip-mode: t
global-eldoc-mode: t
show-paren-mode: t
electric-indent-mode: t
mouse-wheel-mode: t
tool-bar-mode: t
menu-bar-mode: t
file-name-shadow-mode: t
global-font-lock-mode: t
font-lock-mode: t
blink-cursor-mode: t
line-number-mode: t
indent-tabs-mode: t
transient-mark-mode: t
auto-composition-mode: t
auto-encryption-mode: t
auto-compression-mode: t
Load-path shadows:
None found.
Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util text-property-search mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils pcmpl-unix cl-seq
cl-macs vc-git diff-mode easy-mmode vc-dispatcher bug-reference byte-opt
gv bytecomp byte-compile cconv time-date em-unix em-term term disp-table
shell subr-x ehelp em-script em-prompt em-ls em-hist em-pred em-glob
em-extpipe em-cmpl em-dirs esh-var pcomplete comint ansi-color ring
em-basic em-banner em-alias esh-mode eshell esh-cmd generator
cl-loaddefs cl-lib esh-ext esh-opt esh-proc esh-io esh-arg esh-module
esh-groups esh-util rmc iso-transl tooltip eldoc paren electric uniquify
ediff-hook vc-hooks lisp-float-type elisp-mode mwheel term/ns-win ns-win
ucs-normalize mule-util term/common-win tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch easymenu
timer select scroll-bar mouse jit-lock font-lock syntax font-core
term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
faces cus-face macroexp files window text-properties overlay sha1 md5
base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind kqueue cocoa ns
lcms2 multi-tty make-network-process emacs)
Memory information:
((conses 16 71482 9836)
(symbols 48 8490 0)
(strings 32 24424 2081)
(string-bytes 1 760661)
(vectors 16 16661)
(vector-slots 8 215237 12871)
(floats 8 32 63)
(intervals 56 573 57)
(buffers 1000 13))
^ permalink raw reply related [flat|nested] 7+ messages in thread
* bug#57905: [PATCH] Fix eshell directory and executable completion on action t
2022-09-17 22:55 bug#57905: [PATCH] Fix eshell directory and executable completion on action t Daniel Pettersson
@ 2022-09-18 7:40 ` Eli Zaretskii
2022-09-18 10:41 ` Lars Ingebrigtsen
1 sibling, 0 replies; 7+ messages in thread
From: Eli Zaretskii @ 2022-09-18 7:40 UTC (permalink / raw
To: Daniel Pettersson; +Cc: 57905
> From: Daniel Pettersson <daniel@dpettersson.net>
> Date: Sun, 18 Sep 2022 00:55:48 +0200
>
> +(defun eshell--pcomplete-executables ()
> + "Complete amongs 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' t
> + ;; 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)
> + (concat specdir cand)
> + cand))
> + cands))
> + cands)))))
Please don't use 'concat' to create a file name with leading
directories; instead, please use file-name-concat.
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#57905: [PATCH] Fix eshell directory and executable completion on action t
2022-09-17 22:55 bug#57905: [PATCH] Fix eshell directory and executable completion on action t Daniel Pettersson
2022-09-18 7:40 ` Eli Zaretskii
@ 2022-09-18 10:41 ` Lars Ingebrigtsen
2022-09-19 0:31 ` Jim Porter
1 sibling, 1 reply; 7+ messages in thread
From: Lars Ingebrigtsen @ 2022-09-18 10:41 UTC (permalink / raw
To: Daniel Pettersson; +Cc: Jim Porter, 57905
Daniel Pettersson <daniel@dpettersson.net> writes:
> This issue is not present with one completion as further down the call
> stack `completion-file-name-table' is called with action t, which concates
> completion string directory with completion candidate.
>
> Possible solution:
Hm... I'm not that familiar with how this works in eshell myself;
perhaps Jim has some comments -- added to the CCs.
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#57905: [PATCH] Fix eshell directory and executable completion on action t
2022-09-18 10:41 ` Lars Ingebrigtsen
@ 2022-09-19 0:31 ` Jim Porter
2022-09-19 8:22 ` Lars Ingebrigtsen
0 siblings, 1 reply; 7+ messages in thread
From: Jim Porter @ 2022-09-19 0:31 UTC (permalink / raw
To: Lars Ingebrigtsen, Daniel Pettersson; +Cc: 57905
On 9/18/2022 3:41 AM, Lars Ingebrigtsen wrote:
> Daniel Pettersson <daniel@dpettersson.net> writes:
>
>> This issue is not present with one completion as further down the call
>> stack `completion-file-name-table' is called with action t, which concates
>> completion string directory with completion candidate.
>>
>> Possible solution:
>
> Hm... I'm not that familiar with how this works in eshell myself;
> perhaps Jim has some comments -- added to the CCs.
With the caveats that I don't know much about pcomplete (maybe someone
who does would have something interesting to say about this patch?) and
that I haven't built Emacs with the patch, I think the logic here makes
sense.
Some regression tests would be nice though. I've been trying to add
tests as I go through various parts of Eshell, but I haven't looked much
at the "interactive" bits like em-cmpl.el yet. 'eshell-test/forward-arg'
in test/lisp/eshell/eshell-tests.el might make for an ok basis to adapt
into some completion tests (which could probably then go into
.../em-cmpl-tests.el).
That said, I wouldn't object to merging this without regression tests;
it doesn't seem fair to me to expect patch authors to write tests when
the component they're patching doesn't have tests in the first place. :)
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#57905: [PATCH] Fix eshell directory and executable completion on action t
2022-09-19 0:31 ` Jim Porter
@ 2022-09-19 8:22 ` Lars Ingebrigtsen
2022-09-19 20:27 ` Daniel Pettersson
0 siblings, 1 reply; 7+ messages in thread
From: Lars Ingebrigtsen @ 2022-09-19 8:22 UTC (permalink / raw
To: Jim Porter; +Cc: 57905, Daniel Pettersson
Jim Porter <jporterbugs@gmail.com> writes:
> That said, I wouldn't object to merging this without regression tests;
> it doesn't seem fair to me to expect patch authors to write tests when
> the component they're patching doesn't have tests in the first
> place. :)
Thanks.
Daniel, I've now pushed your patch to Emacs 29.
This change was just small enough to apply without assigning copyright
to the FSF, but for future patches you want to submit, it might make
sense to get the paperwork started now, so that subsequent patches can
be applied speedily. Would you be willing to sign such paperwork?
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#57905: [PATCH] Fix eshell directory and executable completion on action t
2022-09-19 8:22 ` Lars Ingebrigtsen
@ 2022-09-19 20:27 ` Daniel Pettersson
2022-09-19 20:35 ` Lars Ingebrigtsen
0 siblings, 1 reply; 7+ messages in thread
From: Daniel Pettersson @ 2022-09-19 20:27 UTC (permalink / raw
To: Lars Ingebrigtsen; +Cc: Jim Porter, 57905
> Please don't use 'concat' to create a file name with leading
> directories; instead, please use file-name-concat.
Ah I had know idea of the existence of that function, that definitely
seams like the better option. Thanks for spreading the word.
> Some regression tests would be nice though. I've been trying to add
> tests as I go through various parts of Eshell, but I haven't looked much
> at the "interactive" bits like em-cmpl.el yet. 'eshell-test/forward-arg'
> in test/lisp/eshell/eshell-tests.el might make for an ok basis to adapt
> into some completion tests (which could probably then go into
> .../em-cmpl-tests.el).
I will take a look :)
> Daniel, I've now pushed your patch to Emacs 29.
>
> This change was just small enough to apply without assigning copyright
> to the FSF, but for future patches you want to submit, it might make
> sense to get the paperwork started now, so that subsequent patches can
> be applied speedily. Would you be willing to sign such paperwork?
Great and sure.
^ permalink raw reply [flat|nested] 7+ messages in thread
* bug#57905: [PATCH] Fix eshell directory and executable completion on action t
2022-09-19 20:27 ` Daniel Pettersson
@ 2022-09-19 20:35 ` Lars Ingebrigtsen
0 siblings, 0 replies; 7+ messages in thread
From: Lars Ingebrigtsen @ 2022-09-19 20:35 UTC (permalink / raw
To: Daniel Pettersson; +Cc: Jim Porter, 57905
Daniel Pettersson <daniel@dpettersson.net> writes:
>> Daniel, I've now pushed your patch to Emacs 29.
>>
>> This change was just small enough to apply without assigning copyright
>> to the FSF, but for future patches you want to submit, it might make
>> sense to get the paperwork started now, so that subsequent patches can
>> be applied speedily. Would you be willing to sign such paperwork?
>
> Great and sure.
Great; here's the form to get started:
Please email the following information to assign@gnu.org, and we
will send you the assignment form for your past and future changes.
Please use your full legal name (in ASCII characters) as the subject
line of the message.
----------------------------------------------------------------------
REQUEST: SEND FORM FOR PAST AND FUTURE CHANGES
[What is the name of the program or package you're contributing to?]
Emacs
[Did you copy any files or text written by someone else in these changes?
Even if that material is free software, we need to know about it.]
[Do you have an employer who might have a basis to claim to own
your changes? Do you attend a school which might make such a claim?]
[For the copyright registration, what country are you a citizen of?]
[What year were you born?]
[Please write your email address here.]
[Please write your postal address here.]
[Which files have you changed so far, and which new files have you written
so far?]
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-09-19 20:35 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-17 22:55 bug#57905: [PATCH] Fix eshell directory and executable completion on action t Daniel Pettersson
2022-09-18 7:40 ` Eli Zaretskii
2022-09-18 10:41 ` Lars Ingebrigtsen
2022-09-19 0:31 ` Jim Porter
2022-09-19 8:22 ` Lars Ingebrigtsen
2022-09-19 20:27 ` Daniel Pettersson
2022-09-19 20:35 ` Lars Ingebrigtsen
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.