company-mode: Prevent unnecessary and unexpected calls to capf hooks

company-mode: Prevent unnecessary and unexpected calls to capf hooks

[João Távora]

Hi there,

This started in https://github.com/company-mode/company-mode/pull/845
and Dmitry suggested I bring it here for comments.

Here's a summary:

In eglot.el I am putting a function, eglot-completion-at-point, into the
special hook completion-at-point-functions.  When called, that function
returns a list according to the protocol and in that list are, also
according to the protocol established by completion-extra-properties,
values for :exit-function and :annotation-function.  Instead of passing
named functions as values, I pass lambdas.  These lambdas capture the
lexical environment of eglot-completion-at-point, attempting to save
useful stuff like how the buffer looked just before the completion
attempt.

Now, when using Emacs's own capf frontend, completion-at-point, the
ensuing mechanism guarantees that the version of those lambdas being
called is always the version returned by **the first call to
eglot-completion-at-point**, even in the case of :exit-function, which
is only called after the completion is performed.  At that point in
time, by definition, the buffer state has changed (and this is why it's
very useful to capture it in a closure).

Dmitry and I differ on the actual reason behind this characteristic of
Emacs's completion-at-point: Dmitry thinks it is accidental (as in: it's
the way it works now, could change in the future). I think this should
be viewed as a formal guarantee, even if is yet unwritten (I think).

Accidental or formal, Company-mode breaks this, because its caching
conditions for an internal function that it uses to get to CAPF data
unconditonally break just before calling :exit-function.  I proposed a
fix to Dmitry that would bring back this guarantee for all intents and
purposes, but Dmitry is hesitant.

Can someone shed some light into this, so either Dmitry can fix it or I
can give up lexical captures in my CAPF :exit-function?

Thanks,
João

Re: company-mode: Prevent unnecessary and unexpected calls to capf hooks

[Stefan Monnier]

[ Not sure where you got the "smonnier" in my email, probably a typo.  ]

> This started in https://github.com/company-mode/company-mode/pull/845
> and Dmitry suggested I bring it here for comments.

[ I actually just replied there a few minutes ago.  So here's a copy of
  what I said there.  ]

I think I see what João is talking about: I don't think describing it as
"the very first call" is quite right, and neither is the lexical
environment really relevant (the two calls to capf may simply return two
different function symbols, with no closures in sight), but indeed,
the :exit-function goes together with the completion-table. If you
insert a completion from the completion table, then call capf you might
very well receive a different completion table with a different
exit-function, so the exit function you should call after inserting
a completion is the one you got when you received the original
completion table, not the one you get when you re-call capf.

So the problem is that the company-backend API doesn't give us the
relevant info: when post-completion is invoked all we know is what has
just been inserted, but the completion data (table and exit-function)
which was used to choose this completion is not available to
company-capf. João's patch is a workaround which takes advantage of the
existence of the cache to try and recover the relevant completion data,
under the assumption that the cache has last been refreshed just before
performing the insertion, so it should happen to contain just the data
we need.

I don't know whether João's patch is the best solution, but at least
I can't think of a better one right now: we could try and de-insert the
text, call capf, and then re-insert the text instead, which would avoid
the reliance on the cache, but that sounds ugly; we could also call capf
from the position right before the insertion which is a bit less ugly
but may sometimes still return incorrect data.

This said, the comment should better explain the problem and make it
explicit that this is a workaround.


        Stefan