Re: master 82c76e3: * lisp/eshell/em-cmpl.el: Try and fix bug#41423

Re: master 82c76e3: * lisp/eshell/em-cmpl.el: Try and fix bug#41423

[Basil L. Contovounesios]

monnier@iro.umontreal.ca (Stefan Monnier) writes:

> branch: master
> commit 82c76e3aeb2465d1d1e66eae5db13ba53e38ed84
> Author: Stefan Monnier <monnier@iro.umontreal.ca>
> Commit: Stefan Monnier <monnier@iro.umontreal.ca>
>
>     * lisp/eshell/em-cmpl.el: Try and fix bug#41423
>     
>     (eshell--complete-commands-list): Rename from `eshell-complete-commands-list`.
>     Return a (dynamic) completion table rather than a list of completions.
>     Use `dolist` and `push`.

Ivy's (admittedly quite limited and buggy) completion hasn't worked
properly since this commit.  I've narrowed it down to the following
change in behaviour.

0. emacs -Q
1. Evaluate:

(let* (dir (clean (lambda () (when dir (delete-directory dir t)))))
  ;; Clean up temporary directory on exit.
  (add-hook 'kill-emacs-hook clean)
  ;; Create temporary parent directory.
  (setq dir (make-temp-file "ivy-" t))
  ;; Create executable file bin/foo under parent directory.
  (let ((exe (expand-file-name "bin/foo" dir)))
    (make-empty-file exe t)
    (set-file-modes exe (file-modes-symbolic-to-number
                         "u+x" (file-modes exe))))
  ;; Start `eshell' in parent directory.
  (let ((default-directory dir))
    (eshell))
  ;; Debug in-buffer completion.
  (setq-local completion-in-region-function
              (lambda (beg end table &optional pred)
                (let* ((str (buffer-substring beg end))
                       (md (completion-metadata str table pred))
                       (all (completion-all-completions
                             str table pred (- end beg) md))
                       (standard-output (messages-buffer))
                       (inhibit-read-only t))
                  (terpri nil t)
                  (pp md)
                  (pp all)))))

2. C-h e
3. ./bi TAB

Before the commit in question, this gave:

  (metadata
   (cycle-sort-function . #[...])
   (category . file)
   (completion--unquote-requote . t))
  (#("bin/"
     0 1 (face completions-common-part)
     1 2 (face completions-common-part arg-end t rear-nonsticky
               (arg-end arg-begin))
     2 3 (face (completions-first-difference)))
   . 2)

And after:

  (metadata (completion--unquote-requote . t))
  (#("bin/" 0 4 (face (completions-common-part))) . 0)

Is this expected, or should some of the old information have been
preserved?

The relevant bug in Ivy[1] is due to its completion-in-region-function
unfortunately relying on the completions-first-difference face to know
how much of the existing buffer text to replace[2].  Do you have any
recommendations for what to use instead?

[1]: https://github.com/abo-abo/swiper/issues/2879
[2]: https://github.com/abo-abo/swiper/issues/1361

Thanks,

-- 
Basil

Re: master 82c76e3: * lisp/eshell/em-cmpl.el: Try and fix bug#41423

[Stefan Monnier]

> 2. C-h e
> 3. ./bi TAB
>
> Before the commit in question, this gave:
>
>   (metadata
>    (cycle-sort-function . #[...])
>    (category . file)
>    (completion--unquote-requote . t))
>   (#("bin/"
>      0 1 (face completions-common-part)
>      1 2 (face completions-common-part arg-end t rear-nonsticky
>                (arg-end arg-begin))
>      2 3 (face (completions-first-difference)))
>    . 2)
>
> And after:
>
>   (metadata (completion--unquote-requote . t))
>   (#("bin/" 0 4 (face (completions-common-part))) . 0)
>
> Is this expected, or should some of the old information have been
> preserved?

No, this is not expected (i.e. a regression) and having
`completions-common-part` on the whole of "bin/" seems like a bug (not
just a regression).


        Stefan