Subject: bug#41423: 27.0.91; eshell file completion in tramp dir is slow (3 minutes) [regression on pretest]

From: Stefan Monnier <[email-masked]>
Date: Tue, 01 Sep 2020 20:31:50 -0400

>> Since in your example you have 2 commands (TAB and RET), that gives you
>> a total of 3.
> Hmmm...  It seems to me that in this case, we're done after the first call
> to pcomplete-completions-at-point,

Usually it depends on whether the completion inserts a separator
(e.g. a space in the case of Eshell/Pcomplete).  Otherwise, point is
still placed "within" (tho probably right at the END position) the
completion area and hence completion is not considered to be finished.
You can refine/tune the completion tables so that completion will
considered as finished in more cases, but that wouldn't solve the
problem in the general case anyway.

>> This design relies on the fact that completion tables can be lazy, so it
>> should always be possible to make the completion-at-point-function very
>> cheap and harmless, so it's OK to call it repeatedly (or even needlessly).
> This is not at all documented AFAICS.  Given that it's a crucial aspect for
> your design to work, it should be.

The lispref says the following:

    The functions on this hook should generally return quickly, since they
    may be called very often (e.g., from @code{post-command-hook}).
    Supplying a function for @var{collection} is strongly recommended if
    generating the list of completions is an expensive operation.  Emacs
    may internally call functions in @code{completion-at-point-functions}
    many times, but care about the value of @var{collection} for only some
    of these calls.  By supplying a function for @var{collection}, Emacs
    can defer generating completions until necessary.  You can use
    @code{completion-table-dynamic} to create a wrapper function:

>>> There is no reason to call pcomplete-completions-at-point when RET
>>> is pressed.
>> If running that function is costly, it's a bug.
> It was not before you declared `pcomplete' obsolete and removed
> `eshell-pcomplete'.

Indeed, making the Pcomplete infrastructure compatible with the standard
completion UI introduced some incompatibilities.

This was true before `pcomplete` was declared obsolete, tho: it has been
true since the introduction of `pcomplete-completions-at-point` (some
time around Emacs-24, IIRC).

> "Provide standard completion using pcomplete's completion tables. Same as
> `pcomplete' but using the standard completion UI."
> It's NOT the same as `pcomplete', it relies on different conditions.
> All completion functions called by `pcomplete-completions-at-point' should
> be checked and possibly changed with this new design.

In practice it works fairly well for most cases, but yes some functions
needed to be changed, and others remain, obviously.

> Given this, why did you declared `pcomplete' obsolete (it would make sense
> to have both a simple mechanism for simple cases and a more complex one for
> more complex cases), and why did you remove `eshell-pcomplete'?

There are several questions in there.  I deprecated `pcomplete` and
`eshell-pcomplete` because AFAIK they didn't have noticeably fewer bugs
than `completion-at-point` any more and neither did they offer any
feature available from `completion-at-point`.

W.r.t simple mechanism for simple cases, I'm not sure what that would
concretely look like in this particular case.

Some motivations for `pcomplete-completions-at-point`:
- make it possible to remove duplicate code that deals with the UI
  aspect of completion (i.e. the `pcomplete` command) rather than the
  core purpose of `pcomplete.el` which is to provide a way to specify
  which completion table applies where on a command line.
- let the `pcomplete` machinery work with the standard UI, which means
  it can also (mostly) obey `completion-styles`.
- let the `pcomplete` machinery work with other UIs such as
  `company-mode`.  I believe this last point is more important now.

> Indeed this patch works.

Thanks.

> But now my question is: whould it not be possible to do this (namely,
> returning a lazy completion table) in one of the pcomplete-* functions (in
> `pcomplete-completions-at-point' itself, or in `pcomplete-completions',
> or...), instead of doing this in the individual functions ultimately called
> by `pcomplete-completions-at-point'?

IIRC it wasn't really easy/possible, no.  E.g. in the patch I sent
I think there's a bug in that a leading * should change the START..END
returned by `pcomplete-completion-at-point-function` so the `glob-name`
computation should be done outside of the `completion-table-dynamic`.

It could have been made a bit simpler and cleaner I guess if I had
decided to "start over from scratch" and force a rewrite of every
pcomplete/<foo> function (basically every such function would have to
return the same kind of info as returned by
`completion-at-point-functions`).

It would not have had any code in common with `pcomplete` any more,
other than the underlying design (which I find very clever&elegant).

        Stefan