[PATCH] em-cmpl: fix completion of command paths

[Nicolas Martyanoff]

Completion was originally broken by 82c76e3.

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.

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,88 +378,73 @@ eshell-complete-parse-arguments
           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
   ;; (bug#41423), so do it lazily.
   (let ((glob-name
-        ;; When a command is specified using `eshell-explicit-command-char',
+         ;; When a command is specified using `eshell-explicit-command-char',
          ;; that char is not part of the command and hence not part of what
          ;; 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)))))
-    (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)))
-               (filepath "") (completions ()))
-          ;; Go thru each path in the search path, finding completions.
-          (dolist (path paths)
-            (setq path (file-name-as-directory
-                        (expand-file-name (or path "."))))
-            ;; Go thru each completion found, to see whether it should
-            ;; be used.
-            (dolist (file (and (file-accessible-directory-p path)
-                               (file-name-all-completions filename path)))
-              (setq filepath (concat path file))
-              (if (and (not (member file completions)) ;
-                       (or (string-equal path cwd)
-                           (not (file-directory-p filepath)))
-                       ;; FIXME: Those repeated file tests end up
-                       ;; very costly over Tramp, we should cache the result.
-                       (if eshell-force-execution
+         (if (and (> (length pcomplete-stub) 0)
+                  (eq (aref pcomplete-stub 0) eshell-explicit-command-char))
+             (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)
+         (let* ((paths (eshell-get-path))
+                (cwd (file-name-as-directory
+                      (expand-file-name default-directory)))
+                (filepath "") (completions ()))
+           ;; Go thru each path in the search path, finding completions.
+           (dolist (path paths)
+             (setq path (file-name-as-directory
+                         (expand-file-name (or path "."))))
+             ;; Go thru each completion found, to see whether it should
+             ;; be used.
+             (dolist (file (and (file-accessible-directory-p path)
+                                (file-name-all-completions filename path)))
+               (setq filepath (concat path file))
+               (if (and (not (member file completions)) ;
+                        (or (string-equal path cwd)
+                            (not (file-directory-p filepath)))
+                        ;; FIXME: Those repeated file tests end up
+                        ;; very costly over Tramp, we should cache the result.
+                        (if eshell-force-execution
                             (file-readable-p filepath)
                           (file-executable-p filepath)))
-                  (push file completions))))
-          ;; Add aliases which are currently visible, and Lisp functions.
-          (pcomplete-uniquify-list
-           (if glob-name
-               completions
-             (setq completions
-                   (append (if (fboundp 'eshell-alias-completions)
-                               (eshell-alias-completions filename))
-                           (eshell-winnow-list
-                            (mapcar
+                   (push file completions))))
+           ;; Add aliases which are currently visible, and Lisp functions.
+           (pcomplete-uniquify-list
+            (if glob-name
+                completions
+              (setq completions
+                    (append (if (fboundp 'eshell-alias-completions)
+                                (eshell-alias-completions filename))
+                            (eshell-winnow-list
+                             (mapcar
                               (lambda (name)
                                 (substring name 7))
-                             (all-completions (concat "eshell/" filename)
-                                              obarray #'functionp))
-                            nil '(eshell-find-alias-function))
-                           completions))
-             (append (and (or eshell-show-lisp-completions
-                              (and eshell-show-lisp-alternatives
-                                   (null completions)))
-                          (all-completions filename obarray #'functionp))
-                     completions)))))))))
+                              (all-completions (concat "eshell/" filename)
+                                               obarray #'functionp))
+                             nil '(eshell-find-alias-function))
+                            completions))
+              (append (and (or eshell-show-lisp-completions
+                               (and eshell-show-lisp-alternatives
+                                    (null completions)))
+                           (all-completions filename obarray #'functionp))
+                      completions)))))))))
 
 (define-obsolete-function-alias 'eshell-pcomplete #'completion-at-point "27.1")
 
-- 
2.38.1