bug#16260: 24.3.50; shell-command-completion and removal of exec-directory from exec-path

bug#16260: 24.3.50; shell-command-completion and removal of exec-directory from exec-path

[Eli Zaretskii]

shell--command-completion-data has this FIXME comment:

  (defun shell--command-completion-data ()
    "Return the completion data for the command at point."
    (let* ((filename (or (comint-match-partial-filename) ""))
	   (start (if (zerop (length filename)) (point) (match-beginning 0)))
	   (end (if (zerop (length filename)) (point) (match-end 0)))
	   (filenondir (file-name-nondirectory filename))
	   (path-dirs (cdr (reverse exec-path))) ;FIXME: Why `cdr'?

As Jarek pointed out in his patch for bug #15461, the doc string of
shell-dynamic-complete-command hints on the reason:

  This function is similar to `comint-dynamic-complete-filename', except that it
  searches `exec-path' (minus the trailing Emacs library path) for completion
  candidates.           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

It talks about "Emacs library path", but actually means the libexec
directory, which is normally the value of exec-directory.

However, just clipping away the last member of exec-path list only
does TRT in the default case.  It will not work if either of the
following happens:

  . The user changes epaths.h to specify more than a single directory

  . EMACSPATH is set in the environment

To get this right, we should record at startup time the value of
exec-path before appending $PATH to it, and then filter out the
directories recorded at that time in shell--command-completion-data.

As a minor nit, the doc string of shell-dynamic-complete-command
should not talk about "library path", which is a non-existing term in
Emacs, but instead mention exec-directory.


In GNU Emacs 24.3.50.187 (i686-pc-mingw32)
 of 2013-12-26 on HOME-C4E4A596F7
Bzr revision: 115758 fgallina@gnu.org-20131226164519-xccapciswuo6gvid
Windowing system distributor `Microsoft Corp.', version 5.1.2600
Configured using:
 `configure --prefix=/d/usr --enable-checking=yes,glyphs 'CFLAGS=-O0
 -gdwarf-2 -g3''

Important settings:
  value of $LANG: ENU
  locale-coding-system: cp1255

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-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
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
M-x r e p o r t - e m <tab> <return>

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message format-spec rfc822 mml
easymenu mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util help-fns mail-prsvr mail-utils time-date tooltip electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel dos-w32 ls-lisp
w32-common-fns disp-table w32-win w32-vars tool-bar dnd fontset image
regexp-opt fringe tabulated-list newcomment lisp-mode prog-mode register
page menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock
font-lock syntax facemenu font-core frame cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese hebrew
greek romanian slovak czech european ethiopic indian cyrillic chinese
case-table epa-hook jka-cmpr-hook help simple abbrev minibuffer nadvice
loaddefs button faces cus-face macroexp files text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote make-network-process w32notify w32
multi-tty emacs)

bug#16260: 24.3.50; shell-command-completion and removal of exec-directory from exec-path

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> shell--command-completion-data has this FIXME comment:
>
>   (defun shell--command-completion-data ()
>     "Return the completion data for the command at point."
>     (let* ((filename (or (comint-match-partial-filename) ""))
> 	   (start (if (zerop (length filename)) (point) (match-beginning 0)))
> 	   (end (if (zerop (length filename)) (point) (match-end 0)))
> 	   (filenondir (file-name-nondirectory filename))
> 	   (path-dirs (cdr (reverse exec-path))) ;FIXME: Why `cdr'?

This code now looks like:

	 (path-dirs
	  ;; Ignore `exec-directory', the last entry in `exec-path'.
          (append (cdr (reverse (exec-path)))
	          (if (and (memq system-type '(windows-nt ms-dos))
                           (not (file-remote-p default-directory)))
                      '("."))))

[...]

> However, just clipping away the last member of exec-path list only
> does TRT in the default case.  It will not work if either of the
> following happens:
>
>   . The user changes epaths.h to specify more than a single directory
>
>   . EMACSPATH is set in the environment
>
> To get this right, we should record at startup time the value of
> exec-path before appending $PATH to it, and then filter out the
> directories recorded at that time in shell--command-completion-data.

I haven't looked further at this (since the FIXME is gone), but it this
something that needs further work?

> As a minor nit, the doc string of shell-dynamic-complete-command
> should not talk about "library path", which is a non-existing term in
> Emacs, but instead mention exec-directory.

This doc string doesn't mention "library path" any more.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#16260: 24.3.50; shell-command-completion and removal of exec-directory from exec-path

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: 16260@debbugs.gnu.org,  Jarek Czekalski <jarekczek@poczta.onet.pl>
> Date: Wed, 20 Jan 2021 03:35:56 +0100
> 
> 	 (path-dirs
> 	  ;; Ignore `exec-directory', the last entry in `exec-path'.
>           (append (cdr (reverse (exec-path)))
> 	          (if (and (memq system-type '(windows-nt ms-dos))
>                            (not (file-remote-p default-directory)))
>                       '("."))))
> 
> [...]
> 
> > However, just clipping away the last member of exec-path list only
> > does TRT in the default case.  It will not work if either of the
> > following happens:
> >
> >   . The user changes epaths.h to specify more than a single directory
> >
> >   . EMACSPATH is set in the environment
> >
> > To get this right, we should record at startup time the value of
> > exec-path before appending $PATH to it, and then filter out the
> > directories recorded at that time in shell--command-completion-data.
> 
> I haven't looked further at this (since the FIXME is gone), but it this
> something that needs further work?

Not if we don't care about the two scenarios I described above.

> > As a minor nit, the doc string of shell-dynamic-complete-command
> > should not talk about "library path", which is a non-existing term in
> > Emacs, but instead mention exec-directory.
> 
> This doc string doesn't mention "library path" any more.

Right, that part was fixed in the meantime.