minad / cape

🦸cape.el - Completion At Point Extensions
GNU General Public License v3.0
608 stars 22 forks source link

Add cape-async-capf #14

Closed minad closed 2 years ago

minad commented 2 years ago

Discussion in #7

minad commented 2 years ago

There are still two things missing:

  1. An async super capf, which merges and parallelizes multiple capfs. However while this is a nice feature which clearly shows the advantages of the async design it is also not practically needed currently. As @dgutov noted grouping async backends in company is rarely done. But it would be nice to add the feature later.
  2. The cape-company-to-capf transformer should be modified such that it does not generate an interruptible capf but an async capf. The async capf can then be used by the cape-async-capf transformer which turns it into a normal capf.

But besides that I think this could work out. Question is - what does it take to support this in Company, such that one could convince Company backend authors to switch to asynchronous capfs? One could add acapf support to company-capf, but then company-capf has to report to the frontend that it got interrupted. I guess company-capf could return a future which does never return in the case of an interrupt and at the same time push a fake input event to unread-command-events. But it is a hack, I guess you will come up with a better solution.

dgutov commented 2 years ago

As @dgutov noted grouping async backends in company is rarely done. But it would be nice to add the feature later.

Grouping async + sync could take advantage of it just as well. The only reason this rarely happens right now is because both LSP backends are not asynchronous.

minad commented 2 years ago

You are right, while the async backend computes in the background, the synchronous backend could compute in the meantime. However there is an ordering problem here, the async backend has to be called first otherwise we just wait for the synchronous backend first and then wait for the async backend afterwards.

dgutov commented 2 years ago

However there is an ordering problem here, the async backend has to be called first otherwise we just wait for the synchronous backend first and then wait for the async backend afterwards.

True. But we either leave that up to the user, or make sure that the combining function calls the async backends from the group first.

minad commented 2 years ago

Hmm, there is one major ugliness about the proposed API. Since completion tables have to respect completion-ignore-case and completion-regexp-list the author of the asynchronous completion table has to capture these variables inside the returned future. Capturing dynamic scope is something which is error prone and easily missed. So in the end it may be easier to just educate people on how to write proper interruptible tables, haha!

One possible way out could be to get rid of the dynamically bound variables like completion-regexp-list, completion-ignore-case etc and add these variables as arguments to the completion table function in order to make it explicit that these variables must be respected.

;; old
(lambda (str pred action)
   ...)

;; new
(lambda (str pred action ignore-case regexp-list)
   ...)

;; if we change the arguments, we could even get rid of the future object then and use cps
(lambda (str pred action ignore-case regexp-list callback)
   ...)

EDIT: It seems to me that the calling convention (lambda (str pred action ignore-case regexp-list) ...) is the most reasonable one. For each function we asyncify we allow future return values and add all relevant dynamic scope state to the argument list. The :company-* functions won't get additional arguments - the only function affected is the table, it seems. If we would add a callback argument instead, we would have to change the argument list of every function which impedes reuse and complicates the upgrade path.

dgutov commented 2 years ago

I don't understand. Why capture instead of looking up the dynamic values whenever the collection "resolves"?

minad commented 2 years ago

I don't understand. Why capture instead of looking up the dynamic values whenever the collection "resolves"?

This won't work since the variables could have changed in the meantime. Look at completion-table-case-fold, which is a completion table transformer.

We can write transformers like the following:

(defun completion-table-modify-dynamic-scope-locally (table)
  (lambda (str pred action)
    (let ((completion-ignore-case t)
          (completion-regexp-list (cons "something" completion-regexp-list)))
      (complete-with-action str pred action))))

Now at the time the future resolves, we have left the scope where completion-ignore-case and completion-regexp-list have been modified. However if no such modifiers are applied it will work, since then the completion-regexp-list and completion-ignore-case are set from the outside only by the completion style - if you take a look at the call chains I've shown.

minad commented 2 years ago

Regarding the completion-ignore-case and completion-regexp-list problems, see also the notes in the code:

https://github.com/minad/cape/blob/acd591b0ec24091b10794e9302bbc5ffc628f3af/cape.el#L388-L399

https://github.com/minad/cape/blob/acd591b0ec24091b10794e9302bbc5ffc628f3af/cape.el#L869-L874

jdtsmith commented 2 years ago

Why can't the async wrapper just be inside the regex-setting wrapper? All the various callbacks etc. will then have the correct dynamic environment.

minad commented 2 years ago

Why can't the async wrapper just be inside the regex-setting wrapper? All the various callbacks etc. will then have the correct dynamic environment.

It does not have to be nested like that necessarily. As soon as you construct an async completion table using a transformer as shown above, the completion-regexp-list and completion-ignore-case will be lost when the future is executed. I even make use of such a transformer for cape-company-to-async-capf!

Furthermore since the STR/PREFIX argument is also passed to the completion table for prefix filtering it makes a lot of sense to also pass the REGEXPS and the IGNORE-CASE. The dynamic variables completion-regexp-list and completion-ignore-case can be considered a historical mistake. You can of course argue to just ignore completion table transformers such that the overall dynamic environment will stay constant across the whole completion computation, but this neglects the fact that completion table transformers are a thing, see completion-table-case-fold, which I mentioned before.

But please read again what I wrote above - essentially you repeated @dgutov's question and I am repeating my answer (maybe with a few more details).

minad commented 2 years ago

I try to clarify this a bit more. In the following there are three example completion tables where a correct future is constructed:

(defun example-table-five-arg (str pred action regexps ignore-case)
  (cons :async
        (lambda (callback)
          (funcall callback
                   (let ((completion-regexp-list regexps)
                         (completion-ignore-case ignore-case))
                     (complete-with-action action '("first" "second" "third") str pred)))
          nil)))

(defun example-table-five-arg2 (str pred action regexps ignore-case)
  (cons :async
        (lambda (callback)
          (funcall callback
                   ;; Introducing a new helper!
                   (complete-with-action-extended '("first" "second" "third")
                                                  str pred action regexps ignore-case))
          nil)))

(defun example-table-three-arg (str pred action)
  (let ((crl completion-regexp-list)
        (cic completion-ignore-case))
    (cons :async
          (lambda (callback)
            (funcall callback
                     (let ((completion-regexp-list crl)
                           (completion-ignore-case cic))
                       (complete-with-action action '("first" "second" "third") str pred)))
            nil))))

example-table-five-arg and example-table-five-arg2 use the updated calling convention, where example-table-five-arg2 is slightly nicer to write with a newly introduced helper similar to complete-with-action. Given these three examples of correct completion tables I would argue that the five argument versions are clearer. They make it explicit that the regexp and ignore case arguments should not be ignored. And if the completion table author does not ignore them, they will be automatically captured inside the future closure. This is what makes this API more robust, the author will intuitively write robust code. Since we are trying to make something robust here, this would be to way to go then. However I dislike the deviation from the argument calling convention and I dislike that this sightly hurts reusability, but not as much as when we would directly use a callback argument for all the functions instead of future return values.

minad commented 2 years ago

One more argument why the five argument calling convention is better - let's say that the backend author has the ability to convert emacs regexps to posix regexps (or maybe a subset of the regexps), then these converted regexps could be passed to the backend process for efficient filtering outside of Emacs. Of course a backend author can already do this with the dynamic variable completion-regexp-list, but if we make it an argument it is much more obvious!

See https://github.com/minad/consult/blob/09e7813a4c42be0ce04c500fda44cee457a35c85/consult.el#L572-L591, where I have a simplistic regexp converter (emacs regexp subset to other types, basic, extended, perl).

minad commented 2 years ago

When we are considering changing the calling convention I may even make a more far-reaching proposal. We use keyword arguments (or a plist) which specifies the filter, with the elements :regexps, :ignore-case, :prefix and :predicate. Since this is what this arguments actually are - they are all filters which are supposed to be applied to the candidates. In particular the PREFIX/STR argument is taken of its special role - it is often used for candidate generation and prefix filtering (see completion-table-dynamic/completion-table-with-cache), which is limiting, since it only works with the basic completion style.

(defun example-table-filter-arg (action &rest filter)
  (cons :async
        (lambda (callback)
          (funcall callback
                   (let ((completion-regexp-list (plist-get filter :regexps))
                         (completion-ignore-case (plist-get filter :ignore-case)))
                     (complete-with-action action '("first" "second" "third")
                                           (plist-get filter :prefix) (plist-get filter :predicate))))
          nil)))
jdtsmith commented 2 years ago

essentially you repeated @dgutov's question and I am repeating my answer

We loved him very much, but my grandfather had the bad habit of repeating the same sentence when you asked for further clarification. Perhaps if two people ask similar questions, you can take that as a sign that the explanation has been insufficient (as I see you have).

Unless I am mistaken (quite possible), if the async wrapper is the absolute innermost wrapper, you do not lose the dynamical variables any transformers set. Perhaps that is too onerous a requirement, and it obviously won't work for UI-wrapping (which is "outermost" by definition), in which case the plist approach might be ok.

But I think, in terms of backend author buy-in — which is key, or else all of this is for naught — you should consider the "least apparent change" principle. I.e. lower the barrier of entry for people to adapt their backends (like lsp-mode, or eglot, or ...) to optional-async, with the fewest structural changes. Small additions like an :async property that non-async UI's just ignore are probably an easier ask than adding new unsupported arguments to the table function. Lowering the barriers like this, even at the expense of other desired features, is probably the right call if this is going to go anywhere.

We could always carefully document the proposed additions, setup a test with a fake "slow" backend, and ask the maintainers of a few popular backends that could benefit from easy, optional async what they think.

All just my opinion, of course.

minad commented 2 years ago

We loved him very much, but my grandfather had the bad habit of repeating the same sentence when you asked for further clarification. Perhaps if two people ask similar questions, you can take that as a sign that the explanation has been insufficient (as I see you have).

Oh I have, don't bother with what I said. However I kind of dislike this endless back and forth in online discussions. I don't mean that personally towards you or this particular discussion. But talking in person may resolve some disagreements and misunderstanding much quicker, or even only clarifications as in this case.

Unless I am mistaken (quite possible), if the async wrapper is the absolute innermost wrapper, you do not lose the dynamical variables any transformers set.

Correct. Note that it is more realistic from the construction that the async wrapper is the outmost wrapper.

Perhaps that is too onerous a requirement, and it obviously won't work for UI-wrapping (which is "outermost" by definition), in which case the plist approach might be ok.

I consider this a too onerous requirement. In particular you don't control what someone is doing to your table. I like to retain composability as my Cape package demonstrates. The idea is that completion tables and capfs can be freely recomposed and wrapped. It would be unfortunate to restrict this.

But I think, in terms of backend author buy-in — which is key, or else all of this is for naught — you should consider the "least apparent change" principle. I.e. lower the barrier of entry for people to adapt their backends (like lsp-mode, or eglot, or ...) to optional-async, with the fewest structural changes.

I agree with that.

Small additions like an :async property that non-async UI's just ignore are probably an easier ask than adding new unsupported arguments to the table function. Lowering the barriers like this, even at the expense of other desired features, is probably the right call if this is going to go anywhere.

I don't understand what your exact proposal is here. What is the point of the :async property?

I pushed the change with the plist/filter argument in https://github.com/minad/cape/pull/14/commits/2b7c6155696a7dddfce7db7377b1c8586d11fe89. I quite like this, since it leads to a cleaned up API. Yes, it violates the principle of avoiding changes. On the other hand it is important to come up with a robust API and for completion tables this means passing the regexp-list and ignore-case as argument is the right way, such that the arguments are part of the lexical scope instead of the dynamic scope, which is not constant.

dgutov commented 2 years ago

Now at the time the future resolves, we have left the scope where completion-ignore-case and completion-regexp-list have been modified. However if no such modifiers are applied it will work, since then the completion-regexp-list and completion-ignore-case are set from the outside only by the completion style - if you take a look at the call chains I've shown.

I suppose that depends on whether completion-table-case-fold has to handle asynchronous tables, or "synchronized" tables only?

If it's the latter, its modification of the aforementioned dynamic variables should cover the lifetime of the request, including the time when the future resolves.

If it's the former... I guess rewriting completion-table-case-fold to apply the variables when the request resolves is not a good idea, like:

(defun completion-table-case-fold (table &optional dont-fold)
  "Return new completion TABLE that is case insensitive.
If DONT-FOLD is non-nil, return a completion table that is
case sensitive instead."
  (cons :async
        (lambda (callback)
          (funcall (cdr (all-completions table))
                   (lambda (res)
                     (let ((completion-ignore-case (not dont-fold)))
                       (complete-with-action action res string pred)))))))

? (The above assumes that the table must be asynchronous).

But isn't it a bad idea because in 90%+ of the cases both variables should be consulted when the request is made (to include them in the request payload, for the background process to handle), rather than when the response arrives?

The exceptions would have to capture their current values and pass them on to the resolution code, but if they are in the minority, perhaps this particular aspect does not require changing.

minad commented 2 years ago

The completion-table-case-fold transformer would not work in its current form for acapfs if we change the calling convention. Indeed it is a bad idea to rewrite the transformer in the form you've given above since then we call all-completions twice. Once directly and once by complete-with-action. It is an undesired double filtering.

But isn't it a bad idea because in 90%+ of the cases both variables should be consulted when the request is made (to include them in the request payload, for the background process to handle), rather than when the response arrives?

That's the point. The regexp should be included with the request payload such that it can be used by the future as soon as it resolves. Therefore I introduced a filter argument to bundle all the filter variables in a common request payload. With the existing calling convention the request payload is split into lexical variables/arguments and dynamic variables which is problematic.

The exceptions would have to capture their current values and pass them on to the resolution code, but if they are in the minority, perhaps this particular aspect does not require changing.

The problem is this: The author of an acapf backend has to handle/capture these variables such that a wrapper applied to the acapf has the freedom to change the variable/filter payload.

If you give me an acapf I want to have the ability to create an acapf from it which is case insensitive. For this you have to write the acapf in one of the ways given in https://github.com/minad/cape/pull/14#issuecomment-981806985. You have to make sure that every part of the request payload is handled, also the dynamic variables. I introduced the filter argument to make it easier for you to write such a correct acapf.

Note that we can cleanup the calling convention even more on the way. Take a look at cape-async-capf where I also change the action argument. The action can be try-completion, all-completions, test-completion, metadata or boundaries - all meaningful symbols, no weird nil, lambda, t as for the existing programmed completion.

So I don't think that changing the calling convention should be avoided at all costs, it has been quite ugly all along. If we use a better filter argument we can also use a better action argument and end up with a completion table which is quite nice in my opinion. It looks almost like a clean slate design ;)

The cleaned up complete-with-action substitute for the new calling convention:

https://github.com/minad/cape/blob/57bb153b913e2281b6be186cfaedb83e11a1f745/cape.el#L430-L438

dgutov commented 2 years ago

Indeed it is a bad idea to rewrite the transformer in the form you've given above since then we call all-completions twice. Once directly and once by complete-with-action. It is an undesired double filtering.

One could probably inline the definition of complete-with-action and then edit it to avoid the double filtering. Anyway, it was just an example.

That's the point. The regexp should be included with the request payload such that it can be used by the future as soon as it resolves. Therefore I introduced a filter argument to bundle all the filter variables in a common request payload. With the existing calling convention the request payload is split into lexical variables/arguments and dynamic variables which is problematic.

What I meant is, the regexp would be included in the payload which goes to the background process (daemon, lsp server, or a one-shot program invocation). Since we already told the server to filter using said values (in some appropriate form that the server understands), we won't need them anymore when the call returns. Though it would be a problem if some code processing the response calls all-completions again (which would apply completion-regexp-list again, the value of which might have changed).

Therefore I introduced a filter argument to bundle all the filter variables in a common request payload. With the existing calling convention the request payload is split into lexical variables/arguments and dynamic variables which is problematic.

We were talking previously about keeping to changes to minibuffer.el to a minimum, right? Perhaps it's a good idea, but it sounds like it will require a lot of changes across the board, if only to update the calling conventions. And then, if someone introduces another global var which affects how completion works, that would all be for naught.

Note that we can cleanup the calling convention even more on the way. Take a look at cape-async-capf where I also change the action argument. The action can be try-completion, all-completions, test-completion, metadata or boundaries - all meaningful symbols, no weird nil, lambda, t as for the existing programmed completion.

Sounds like the Big Redesign kind of affair (one which was also previously suggested to use generic functions, in another discussion). I can't comment on that now, except to say it should probably be discussed on emacs-devel. Or in private (or even here, maybe), but with other stakeholders included as well.

The problem is this: The author of an acapf backend has to handle/capture these variables such that a wrapper applied to the acapf has the freedom to change the variable/filter payload.

But as long as the variables are applied during the request phase (not during handling of the response), passing them in to an acapf backend should be easy enough, like in my example in the previous message.

Where is could become more complicated, is in a "chain of futures" kind of situation. But then, I'm not sure how realistic that is, and what exact behavior we would really want there.

dgutov commented 2 years ago

Another, a more far-out approach would be to try the threads (like previously mentioned/hinted in the other issue) as the vehicle of async computations. They should be able to make stacking of computations easier (since that just looks like plain function composition), and a local variable binding in a thread could span multiple requests-responses.

But it's much more of a research project than a concrete proposal.

minad commented 2 years ago

Regarding threads I am fairly critical. I think the async design with a single thread is better. Using threads with a gil for concurrency is an ugly model for my taste.

For concurrency with parallelism I favor a model where elisp gets separate worker jobs with separate heaps which communicate via channels - a similar design to JavaScript and workers. The workers and the main thread can also communicate via async futures.

dgutov commented 2 years ago

It could work (probably) like Fibers in Ruby. If we had a counterpart to the newly introduced FiberScheduler.

That piece of code could, similarly to our sit-for loop, see when requests are sent and when they arrive, and abort and clean up everything at an appropriate point.

I'm not sure how I feel about this, really, without seeing a prototype patch, but a fiber-based design is one that could both avoid a lot of changes in the code (the styles' implementations could continue to work as-is and stay ignorant about all of the context switching), and offer a "true" parallelism for completion tables. To the extent that Emacs allows us, of course.

A futures based design should feel more familiar to more programmers, though. Because JavaScript.

minad commented 2 years ago

The problem with fibers/green threads is that it will not magically turn existing blocking completion tables into cooperative completion tables without changes to the elisp runtime/interpreter. Every blocking operation should be turned into a non-blocking operation behind the scene plus a context switch. I am unsure how realistic this is given the Emacs code base.

There is precendence though, the ruby fiberscheduler that you mentioned and I think java will also get fibers at some point where all blocking operations are made non-blocking behind the scenes.

minad commented 2 years ago

An async design integrates more gracefully into an existing single threaded system like Js or a system where green threads are too expensive like rust. By providing additional async syntax (basically monadic desugaring) one ends up with code that looks like blocking code, similar to the seemingly blocking green thread code. So the difference is that fo async the problem is solved on the language level (macros!) and for green threads the problem is solved on the runtime level.

jdtsmith commented 2 years ago

While I agree that capf is not something anyone would design now, and due to its accumulated layers is quite challenging to reason about, it seems to me unlikely all the capf code in the world will get re-written en masse, even if superior async support and a modern new completion API ended up in emacs core.

I still feel like there must be a simple route, in the meantime, to add optional complete/cancel callback passing on top of capf without breaking too much. More than just completion UI's would love to have async functionality, so I don't think the risk of taking wind out of the sails en route to the bright land of async is too severe.

My interest has become a bit more pressing, as I've now discovered a variety of completion backend commands that can block unexpectedly for multiple seconds. I could do my own wrapping with while-no-input inside the capf table-updating code, to isolate interruption to the hotspot location where it is most needed (accept-process-output loop), and then clean up afterwards, but doing that in the backend seems like a strange place to be worrying about the timing of input.

minad commented 2 years ago

While I agree that capf is not something anyone would design now...

I disagree with this statement. The Capf/completion table design is not that unreasonable, even if Dmitry and you believe so. I've worked with this a while and the APIs certainly have their rough edges and ugliness, but the general idea is not unreasonable. It is understandable that Dmitry dislikes the existing design, since it does not fit nicely together with the Company API resulting in an ugly adapter. But this only shows that there is a mismatch between the APIs, not that the original Capf/table API is bad.

  1. The Capf is a basic query mechanism which is used to ask if completion is possible at the given point. Then a completion table is returned which is called by the UI to perform the actual completion.
  2. The completion table has actions (try, test, all), which can be used to expand the current input, to test the current input and to return all matching completions respectively. All these actions are reasonable. There are also special actions like boundaries and metadata. In particular boundaries is a complicated one, but it is only rarely needed when completing substrings.
  3. The completion table receives the filter arguments PREFIX, IGNORE-CASE, REGEXP-LIST and PREDICATE which allow the completion table (backend) to efficiently pre-filter the returned candidates. The actions try, test and all must respect these arguments.
  4. Completion styles are layered on top to support flexible filtering. The completion styles do nothing else than calling the table with the aforementioned arguments. Essentially the completion style compiles the input to PREFIX and REGEXP-LIST, see for example Orderless. Note that you don't have to use completion styles as filter mechanism, you can devise your own. It makes sense to separate backend filtering and frontend filtering, it gives us a hell of flexibility shown by the various available completion styles.
  5. By separating Capfs and tables it is possible to reuse the completion table API for both completion at point and completing-read. This is a good idea since it allows you to trivially transfer the Corfu completion to the minibuffer, see for example https://github.com/minad/corfu/wiki#transfer-to-the-minibuffer.

Now one can ask what is actually wrong with this design. I like this reusability aspect of the API, but one could also conflate Capf and completion table ending up with a design like Company, with additional actions like prefix. I don't think that's better than the two-step Capf process. Another point of the existing API which may be considered problematic is the extensibility. In order to add annotations and other metadata the completion metadata is extended with additional functions. I don't like this, it seems simpler to have an API like company, where the completion table can simply be extended with additional actions (annotation, deprecated, kind etc). On the other hand the completion table design with its annotation-function metadata etc, encourages reuse of these auxillary functions. But overall I consider the extension issue a minor issue which does not invalidate the overall design.

I still feel like there must be a simple route, in the meantime, to add optional complete/cancel callback passing on top of capf without breaking too much. More than just completion UI's would love to have async functionality, so I don't think the risk of taking wind out of the sails en route to the bright land of async is too severe.

I've shown such a route. My proposal achieves a number of goals I've set. It integrates cleanly with the existing code base. It reuses the entire existing functionality. It supports async completion tables and it allows you to combine multiple async completion tables which are queried in parallel. Furthermore by consolidating the filter arguments in a plist (prefix, predicate, regexp-list, ignore-case), the backend stays extensible and more filters can be added in the future. I argue that one ends up with a cleaned up backend API with async support, which is an improvement over the status quo. Since the new API is very close to the existing API, backends can be ported easily to the new API. Since I implemented the mechanism as part of my Cape package in the PR, integration into the existing system is possible. No upstream changes are needed (to Corfu, default completion). This does of course not preclude the direct implementation of async tables in Corfu or default completion on a later stage.

Unfortunately the discussion reached a dead end here and it seems unlikely that we will reach consensus, since Dmitry does not like the existing API from the get go and wants to go with a big rewrite. I disagree with such an approach for multiple reasons. 1. the existing API is not unreasonable, 2. by doing a big rewrite one introduces a lot of compatibility problems and has a hard time to retrofit existing code to a hypothetical new API and 3. the proposal of a hypothetical new API is not concrete - how should it look? Dmitry made a few arguments which are of course relevant issues that must be addressed, but all of those are easily solvable. There are no blockers. Therefore I don't consider the discussion here particularly constructive or effective. If there does not exist consensus regarding the starting point one can always dig out many minor arguments which seemingly block the proposal. My PR shows that the proposal of an async capf can be implement, the arguments against it are details and refinements but not real blockers.

I consider myself to be a "formalist" regarding code base refactoring, this means I am okay with working with existing APIs and formally mapping them to new constructs and vice versa, which guarantees me that existing functionality does not break. I prefer to address small pain points one by one, refining the existing functionality step by step. By doing small formal transformation steps it is guaranteed that no bugs are introduced and that no existing functionality is lost on the way.

Anyway there is no right or wrong here. Ultimately it boils down to preferences. However I don't see a chance for such a proposal to succeed if Dmitry disagrees with it, I am not even speaking of convincing backend authors. I could offer an aysnc capf API as part of Cape, but this wouldn't be a bit better than inventing my own incompatible API, similar to how company and autocomplete have their own APIs. Therefore for now I think it is better to simply stick to the existing synchronous Capf completion API in Corfu/Cape.

jdtsmith commented 2 years ago

Very useful summary thanks.

The Capf is a basic query mechanism which is used to ask if completion is possible at the given point. Then a completion table is returned which is called by the UI to perform the actual completion.

Most/all backends which gather completions from external or network-attached processes will not know this, or at least not know which portion of the buffer it can/will complete/replace. That to me is the primary limitation. Not insurmountable, but never fully/cleanly solvable with the two-step capf design. Another limitation is that the CAPF design expects the full and complete list of completions with all associated data to be present "on demand", and has no means of gradual delivery. But I agree with all your sentiments on positive incremental change.

Since the new API is very close to the existing API, backends can be ported easily to the new API.

Since I implemented the mechanism as part of my Cape package in the PR, integration into the existing system is possible. No upstream changes are needed ...

IMO, these are two very compelling arguments. Is there any reason then not to merge your PR and let async backend authors use CAPE (or their own "sync-ifiers" based on it) to present a sync interface to corfu/company/default completion UIs? The fact that company may not implement its own handling of ACAPFs isn't so severe a limitation. Once wrapped, ACAPFs become fully API-compliant CAPFs, yes? UI's are none the wiser.

So it's not really inventing your own incompatible API, rather providing an "internal"/utility API for mapping callback-handled ASYNC backends -> capf backends. And if ACAPFs become popular because they perform better, work with all UI's, and are easier to code, etc., the situation may change, and UIs may start supporting them directly. But I suspect that last step isn't essential to provide real utility to backends with slow & variable data providers.

minad commented 2 years ago

Most/all backends which gather completions from external or network-attached processes will not know this, or at least not know which portion of the buffer it can/will complete/replace. That to me is the primary limitation. Not insurmountable, but never fully/cleanly solvable with the two-step capf design.

Well, the current completion API design has no support for asynchronous retrieval of candidates. Therefore we discuss here. However I don't see what a two-step or single-step design would change with regards to supporting asynchronous completion. If you have a single step you would still ask the completion backend if it can complete at the given point. In the worst case the backend always has to pretend that it can complete at the given point, basically collapsing the two steps into a single step. But I suspect that one can do better in many scenarios using heuristics. The two-step design is a result of the reuse of the completion table API, but it does not preclude an asynchronous extension.

Another limitation is that the CAPF design expects the full and complete list of completions with all associated data to be present "on demand", and has no means of gradual delivery.

I am not convinced that I want that, if by gradual delivery you mean something like the dynamically incoming candidate batches as in consult-grep. It is good to have this for consult-grep but I don't want the UI to populate gradually while completing within the buffer. On the other hand, asynchronous futures (all candidates or nothing) are a useful addition. They have two advantages, 1. we can query backends in parallel and 2. we don't have to rely on interrupts to retain responsiveness.

IMO, these are two very compelling arguments.

Indeed. But not got enough to find consensus here. Rather the advantages are disadvantages from Dmitry's perspective since they stabilize and calcify the existing structures.

Is there any reason then not to merge your PR and let async backend authors use CAPE (or their own "sync-ifiers" based on it) to present a sync interface to corfu/company/default completion UIs? The fact that company may not implement its own handling of ACAPFs isn't so severe a limitation. Once wrapped, ACAPFs become fully API-compliant CAPFs, yes? UI's are none the wiser.

In comparison to the existing cape-capf-* transformers a cape-async-capf transformer introduces a new API. If backend authors start to use this API we just end up with four different APIs, capf, company, autocomplete and cape-acapf but it does not improve the status quo if the Company maintainers explicitly have no interest in supporting such an API. It only makes sense to introduce a new API if there is a certain momentum behind it, which is not the case.

So it's not really inventing your own incompatible API, rather providing an "internal"/utility API for mapping callback-handled ASYNC backends -> capf backends.

This is hair splitting. I argue that ACAPFs introduce a new API, even if they can be implemented as a transformation.

And if ACAPFs become popular because they perform better, work with all UI's, and are easier to code, etc., the situation may change, and UIs may start supporting them directly.

This may be true. But my interest in such a "competitive approach" is zero. I prefer collaboration and finding agreement.

But I suspect that last step isn't essential to provide real utility to backends with slow & variable data providers.

Sure, a backend author can use such an async-based implementation within the backend. But I don't have interest in maintaining this functionality here. Note that Dmitry is a core contributor and I am not contributing to the core. I don't want to push ideas here against Dmitry's opinion which have only a negligible chance of getting adopted. Furthermore when proposing acapfs to backend authors the main selling point would be to have an API which works equally well in Company and Corfu. If this is not the case, there is no point. My main interest is in supporting existing backends and this goal has mostly been reached thanks to the cape-company-capf adapter and the other adapters.

See my comment https://github.com/minad/cape/issues/7#issuecomment-980476232. We are stuck in the beginning.

minad commented 2 years ago

@jdtsmith

My interest has become a bit more pressing, as I've now discovered a variety of completion backend commands that can block unexpectedly for multiple seconds. I could do my own wrapping with while-no-input inside the capf table-updating code, to isolate interruption to the hotspot location where it is most needed (accept-process-output loop), and then clean up afterwards, but doing that in the backend seems like a strange place to be worrying about the timing of input.

Btw, which are the backends you are talking about here? If you discover issues with existing backends please tell me. Then we can see if we can address the issues on a case by case basis. We should also document this in the Corfu wiki.

jdtsmith commented 2 years ago

Right now my main focus and use case is on a backend I'm authoring which interacts with the same ipython process the user interacts with, hiding some of the output. So it's very sensitive to delays. I may try the "local input interruption of process output" approach, i.e. interrupt myself instead of letting Corfu do it "from above". This may still lead to some races with prompt arrival. Even LSP mode has it easier, with its separate standalone process channel.

Old style completion was just so much easier: M-Tab and wait (and wait) for the results in a separate buffer. But not really state of the art.

dgutov commented 2 years ago

I disagree with this statement. The Capf/completion table design is not that unreasonable, even if Dmitry and you believe so. I've worked with this a while and the APIs certainly have their rough edges and ugliness, but the general idea is not unreasonable. It is understandable that Dmitry dislikes the existing design, since it does not fit nicely together with the Company API resulting in an ugly adapter. But this only shows that there is a mismatch between the APIs, not that the original Capf/table API is bad.

As I said before, I think its general shape is reasonable, even if a number of details could be written in way that are easier to follow and understand for a casual contributor. And I wouldn't say company-capf is that ugly.

By separating Capfs and tables it is possible to reuse the completion table API for both completion at point and completing-read. This is a good idea since it allows you to trivially transfer the Corfu completion to the minibuffer, see for example

It might be better to transition to the world where completing-read uses CAPF objects, rather than completion tables. Because that makes the latter more complex (e.g. requires the introduction of the concept of "fields").

Now one can ask what is actually wrong with this design. I like this reusability aspect of the API, but one could also conflate Capf and completion table ending up with a design like Company, with additional actions like prefix. I don't think that's better than the two-step Capf process.

I can't really say whether one is better than the other, but it would be nice if someone really tried to move CAPF to a "conflated" design, and then described the problems and practical limitations that come from that.

It's interesting to note that seemingly all alternative completion packages with their own source format used the latter approach: auto-complete, company and the seemingly-more-extensible-though-not-really completion-ui (which see). Perhaps they had more limited goals. But perhaps the difference between CAPF and a completion table was more about backward compatibility where it was, again, easier to add a wrapper than to do a rewrite.

Anyway there is no right or wrong here. Ultimately it boils down to preferences. However I don't see a chance for such a proposal to succeed if Dmitry disagrees with it, I am not even speaking of convincing backend authors.

I should clarify: I'm not crazy about the proposal, but don't want to block it either. Due a number of factors, however, it doesn't seems feasible to do a "full" support/integration in company-capf (as explained in the other issue). But even a minimal integration (like setting cape-interrupt-symbol var to a known value and then catching it in the wrapper) can allow the users of Company to use this modification. Even though, to take advantage of parallelism in execution they'd have to forgo the built-in "grouping" feature and instead go CAPF-only, grouping those using your combinators.

And then, if this approach turns out to be popular enough, there are a few directions to go from there. Either try to bring "proper" async CAPF to the core (using the same new calling convention, but without the "interrupting" wrapper), or have Company drop all non-CAPF backends (sometime in distant-ish future). Or both.

minad commented 2 years ago

It might be better to transition to the world where completing-read uses CAPF objects, rather than completion tables. Because that makes the latter more complex (e.g. requires the introduction of the concept of "fields").

Why? Capfs don't make sense in the context of completing-read. I would even say it is more realistic to remove Capfs and use completion tables for everything by adding a prefix action to completion tables, similar to what Company has. This would also get rid of the two-step approach.

It's interesting to note that seemingly all alternative completion packages with their own source format used the latter approach: auto-complete, company and the seemingly-more-extensible-though-not-really completion-ui (which see). Perhaps they had more limited goals. But perhaps the difference between CAPF and a completion table was more about backward compatibility where it was, again, easier to add a wrapper than to do a rewrite.

Yes, this is the case. But the layering is also not a bad idea per se, the two-step process is meaningful. Anyways if one would get rid of the two step process one should rather use completion tables as the only entity that will remain, augmented with additional actions like prefix for the initial query of the completion position. But all this is quite a bit away from the existing design.

I should clarify: I'm not crazy about the proposal, but don't want to block it either. Due a number of factors, however, it doesn't seems feasible to do a "full" support/integration in company-capf (as explained in the other issue).

I am not so sure about that. A full integration seems possible, if by full integration we mean something which is as good as the current async company backends. Of course if we move the goalpost to something where the async operations are scheduled more freely, then this is not possible since the wrapper will force synchronization. However this seems to go beyond what Company does now.

But even a minimal integration (like setting cape-interrupt-symbol var to a known value and then catching it in the wrapper) can allow the users of Company to use this modification.

Indeed. This would be a first step.

Even though, to take advantage of parallelism in execution they'd have to forgo the built-in "grouping" feature and instead go CAPF-only, grouping those using your combinators.

This is correct if only such a minimal integration is done. However if one implements a more intrusive integration it is possible to group async capfs and async company backends all together. I described such a possible synchronization design before. The entire synchronization would happen in the capf wrapper, given a list of futures to wait for. After the synchronization finished the grouping code can access all the results which are available then.

And then, if this approach turns out to be popular enough, there are a few directions to go from there. Either try to bring "proper" async CAPF to the core (using the same new calling convention, but without the "interrupting" wrapper), or have Company drop all non-CAPF backends (sometime in distant-ish future). Or both.

Yes, this goes in the direction of what @jdtsmith said.

I should clarify: I'm not crazy about the proposal, but don't want to block it either.

Okay, let's think a bit more about this. I see your points and also have my doubts about all this. I think it is all solvable on the technical level, but I am unsure about adoption and if we really want to end up with an API of this form. By implementing it first in Cape only and then asking backend authors if they are interested in implementing this API we are certainly not pushing for adoption, so this will probably just go nowhere. I guess I am also not crazy about this.

Another idea I've considered is to use the Company backend API and make this more of a first class citizen in the Capf framework. In the main branch I have cape-company-capf which synchronizes the company backend turning it into an interruptible capf. What is lacking is the combinator to group multiple company backends to form a super company backend, such that parallelization becomes possible.

While I am not a big fan of the Company backend API, I assume that it is good enough for most completion needs (as proven by Company). The only thing lacking are completion boundaries and probably some other oddities of the completion table API. But these features are not used often for in-buffer completion (Corfu supports file name completion via cape-file, which makes use of boundaries but that's about it).

Unfortunately there is one "design flaw" in the Company backend API. Often the Company backends actually require Company to implement the interactive action and to retrieve the thing at point. I've looked into a handful of Company backends and all of them make only very light use of Company functionality and the Company dependency could be removed easily. Of course the Company dependency is not a blocker to use the backends with other frontends. The Company backends work with cape-company-capf. However thanks to the dependency one cannot really consider the API and the backends as agnostic and independent of the frontend.

dgutov commented 2 years ago

Why? Capfs don't make sense in the context of completing-read. I would even say it is more realistic to remove Capfs and use completion tables for everything by adding a prefix action to completion tables, similar to what Company has. This would also get rid of the two-step approach.

These two options seem functionally equivalent (prefix ~~ boundaries). Except I'm not quite sure how to best implement the file name expansion like ~/v/em/src/ to ~/vc/emacs/src while displaying only base file names in completions list. But looking at what icomplete-mode shows as hints in this scenario, the key is probably doing a bit more work in the prefix handler.

Yes, this is the case. But the layering is also not a bad idea per se, the two-step process is meaningful. Anyways if one would get rid of the two step process one should rather use completion tables as the only entity that will remain, augmented with additional actions like prefix for the initial query of the completion position.

Maybe the best choice is dependent on the availability of existing functions that can be used as completion tables (if the prefix might vary depending on the context). Looking at it like that, the CAPF<->CT relation might not be that bad.

A full integration seems possible, if by full integration we mean something which is as good as the current async company backends. Of course if we move the goalpost to something where the async operations are scheduled more freely, then this is not possible since the wrapper will force synchronization.

The former sounds like the "minimal" integration I described. Which shouldn't (which is a good thing) require much effort on my part.

However this seems to go beyond what Company does now.

No, that would be what Company does now with native async backends. It's just that neither eglot nor lsp-mode take advantage of this.

This is correct if only such a minimal integration is done. However if one implements a more intrusive integration it is possible to group async capfs and async company backends all together. I described such a possible synchronization design before.

Perhaps I didn't understand it.

The entire synchronization would happen in the capf wrapper, given a list of futures to wait for. After the synchronization finished the grouping code can access all the results which are available then.

That's the approach where grouping has to happen on the level of CAPF API, so no native Company backends would be able to run in parallel, right?

Either way, I don't think any additional changes would be required inside company-capf if your grouping wrapper does all this. And then gets wrapped by a synchronizer.

By implementing it first in Cape only and then asking backend authors if they are interested in implementing this API we are certainly not pushing for adoption, so this will probably just go nowhere.

If the new extension is sound, then making it usable in more places could drive adoption (especially since the current way the "interruptable" CAPF backends are written is pretty error-prone), which could create more motivation to add it in the core more properly. But :shrug:. It would be an experiment either way.

Another idea I've considered is to use the Company backend API and make this more of a first class citizen in the Capf framework. In the main branch I have cape-company-capf which synchronizes the company backend turning it into an interruptible capf.

Interesting turn, even if it's also not ideal probably, given that the "official" stance on the Company backends is "no". But they seem a little easier to write OTOH, and could suffice for the immediate needs.

What is lacking is the combinator to group multiple company backends to form a super company backend, such that parallelization becomes possible.

You might be able to reuse company--multi-backend-adapter instead of having to rewrite it.

The only thing lacking are completion boundaries and probably some other oddities of the completion table API. But these features are not used often for in-buffer completion (Corfu supports file name completion via cape-file, which makes use of boundaries but that's about it).

Yeah, it's a known omission, and I wasn't sure how to add/implement/name is best. Would it be perhaps possible to implement adequate file name completion without it, though? See above.

Unfortunately there is one "design flaw" in the Company backend API. Often the Company backends actually require Company to implement the interactive action and to retrieve the thing at point. I've looked into a handful of Company backends and all of them make only very light use of Company functionality and the Company dependency could be removed easily.

It's more of a convention than a strict requirement, given that the only time it is used is when the backend is invoked as a command interactively. Some backends actually are actually missing it. But OTOH it's been handy for a lot of users with e.g. company-yasnippet. I'm not sure how to resolve this: no problem with saying that it's not a necessary part of a backend definition, but some authors will probably include it anyway (unless we explicitly prohibit that, for some reason), and then you'll have some packages/backends depending on company anyway.

minad commented 2 years ago

Thanks for your answer!

These two options seem functionally equivalent (prefix ~~ boundaries). Except I'm not quite sure how to best implement the file name expansion like ~/v/em/src/ to ~/vc/emacs/src while displaying only base file names in completions list.

Boundaries have prefix and suffix, but besides that they seem equivalent. The difference with the Capf design is that the prefix is determined at startup and during the remaining completion the boundaries are the relevant. For Company backends you could probably replace the prefix action with a boundary action to achieve feature parity.

That's the approach where grouping has to happen on the level of CAPF API, so no native Company backends would be able to run in parallel, right?

No, it would still be possible to run ACapf and Company backends in parallel. However the entire synchronization/waiting for all backends would have to happen inside the ACapf synchronization. It is a bit ugly, but doable. I guess I didn't explain this well enough. The idea is that the company-capf backend would be the synchronizer itself. There is no separate synchronizer wrapper for such a full integration with Company. For the "light integration" one could just go with the synchronizer wrapper, but then no parallelization with Company backends is be possible, that's all.

If the new extension is sound, then making it usable in more places could drive adoption (especially since the current way the "interruptable" CAPF backends are written is pretty error-prone), which could create more motivation to add it in the core more properly. But shrug. It would be an experiment either way.

Yeah, so that's the question if I really want to go that route and propose a new experimental Capf API which splits the ecosystem further. Generally I am not a fan at all of fragmentation, in particular not on the API level.

(While it may look as if the new completion packages fuel fragmentation, the motivation is the opposite. All the packages are relying on common APIs resulting in a coherent whole instead of creating separate ecosystems.)

Interesting turn, even if it's also not ideal probably, given that the "official" stance on the Company backends is "no". But they seem a little easier to write OTOH, and could suffice for the immediate needs.

Indeed they seem easier to write. The only thing which prevents me from getting behind such an API is that most backends require Company. I've looked around but I've not found a single one which does not depend on Company, even if it is easy to write an agnostic backend as shown in the small snippet from the Cape README (https://github.com/minad/cape/#company-adapter).

So in order to transcend from a Company-only to a general async (and frontend agnostic) API a few changes on the side of Company would be required and you are probably not gonna like this :-P

First company-begin-backend and company-other-backend should be deprecated in order to prevent backends from unnecessarily depending on Company for its interactive action. The interactive action should be discouraged. I am not yet sure on how to retain the functionality, since there is some value in the possibility of calling backends interactively.

Then the company-grab-* functions should be deprecated (or made private) such that backends stop relying on them. Instead backends should implement this logic on their own, maybe relying on thing-at-point etc.

Overall the situation with Company backends is not bad, via cape-company-capf one can use them as is. And cape-company-capf is only a thin adapter. All I am saying is that the Company API is unnecessarily coupled to Company (backend-frontend coupling) which is not friendly to other frontends. While the Company API is in principle agnostic (which is good) it does not look like this. The API rather gives the impression that it is "only" a Company plugin API and not something which could also fly in other contexts.

You might be able to reuse company--multi-backend-adapter instead of having to rewrite it.

Ah lol, so you already implemented multi backends as an adapter too. This is exactly the design I had in mind. So indeed, no reimplementation of that part is required. Probably it will even work already in Corfu if one groups multiple async backends with company--multi-backend-adapter and wraps the result with cape-company-capf.

EDIT: Indeed this works already.

(setq completion-at-point-functions
      (list
       (cape-company-to-capf
        (apply-partially #'company--multi-backend-adapter
                         (list #'company-dabbrev #'company-elisp)))))

Yeah, it's a known omission, and I wasn't sure how to add/implement/name is best. Would it be perhaps possible to implement adequate file name completion without it, though? See above.

If I understood correctly you use a special file backend in Company which generates the full paths?

It's more of a convention than a strict requirement, given that the only time it is used is when the backend is invoked as a command interactively. Some backends actually are actually missing it. But OTOH it's been handy for a lot of users with e.g. company-yasnippet. I'm not sure how to resolve this: no problem with saying that it's not a necessary part of a backend definition, but some authors will probably include it anyway (unless we explicitly prohibit that, for some reason), and then you'll have some packages/backends depending on company anyway.

Yes. As I wrote above if one discourages/deprecates/replaces the problematic APIs in Company one could encourage authors of existing backends to drop the Company dependency. I like if packages are decoupled and if I can recompose them at will, and in this case it means that backends are agnostic with respect to the frontend. But it may not be in your interest to take Company out of the equation ;)

dgutov commented 2 years ago

Boundaries have prefix and suffix, but besides that they seem equivalent. The difference with the Capf design is that the prefix is determined at startup and during the remaining completion the boundaries are the relevant. For Company backends you could probably replace the prefix action with a boundary action to achieve feature parity.

Ah, yes, Prefix -> prefix+suffix, or a pair of positions, similarly to CAPF's start and end. Though some question remains on how this should affect the behavior (replace the suffix, post-filter using it, and/or avoid idle completion when inside a word). https://github.com/company-mode/company-mode/issues/340#issuecomment-921464812

No, it would still be possible to run ACapf and Company backends in parallel. However the entire synchronization/waiting for all backends would have to happen inside the ACapf synchronization. It is a bit ugly, but doable. I guess I didn't explain this well enough. The idea is that the company-capf backend would be the synchronizer itself.

I suppose this is possible to do, at least in theory. Sounds a bit messy and error-prone to implement, though.

(While it may look as if the new completion packages fuel fragmentation, the motivation is the opposite. All the packages are relying on common APIs resulting in a coherent whole instead of creating separate ecosystems.)

And I get that motivation: if the async CAPF could be written similarly enough to simple CAPF, that would allow the providers to avoid modifying the code much, and perhaps support both conventions with little effort. Same would go for frontends, hopefully.

First company-begin-backend and company-other-backend should be deprecated in order to prevent backends from unnecessarily depending on Company for its interactive action. The interactive action should be discouraged. I am not yet sure on how to retain the functionality, since there is some value in the possibility of calling backends interactively.

Suggestions welcome on how to retain it. It's a handy functionality, isn't it?

Then the company-grab-* functions should be deprecated (or made private) such that backends stop relying on them. Instead backends should implement this logic on their own, maybe relying on thing-at-point etc.

Same goes for this. And since we're talking in the context of cape, I suppose you would want to provide the alternatives for those function in this package?

That aside, I was going to use company-grab-symbol to "transparently" migrate the return value of prefix from a string to a cons of numbers someday. Though maybe a suffix command would provide a better transition, given that prefix can already return a cons, and a list there would be kind of messy.

The API rather gives the impression that it is "only" a Company plugin API and not something which could also fly in other contexts.

I suppose we can go and try to "free" that API, if we decide it is functional enough, and there is a demand for it to be used across frontends. We should really bring @monnier into those discussions, though.

Ah lol, so you already implemented multi backends as an adapter too. This is exactly the design I had in mind. So indeed, no reimplementation of that part is required. Probably it will even work already in Corfu if one groups multiple async backends with company--multi-backend-adapter and wraps the result with cape-company-capf.

Note that that adds a dependency on company, though. ;-(

If I understood correctly you use a special file backend in Company which generates the full paths?

Yes: but it doesn't look nice when the full file name gets long enough to span the whole width of the window: https://github.com/company-mode/company-mode/issues/1040

I suppose a good test of this alternative would be to figure fixing this problem without bringing in the notion of "fields" from CAPF. Which I was kind of going to use, after I "figure it out properly".

I like if packages are decoupled and if I can recompose them at will, and in this case it means that backends are agnostic with respect to the frontend. But it may not be in your interest to take Company out of the equation ;)

Not really a problem for me, but I worry about backend developers feeling whiplash about the change in strategy (company? capf? company again?), and about being able to retain the niceties that the dependencies that you mentioned provide.

company-grab-symbol could probably be replaced with bounds-of-things-at-point if we change/rename prefix into a method returning a start-end cons. No obvious alternative for `company-begin-backend, though.

I suppose we could have a separate macro like company-define-backend-command, but then the backend authors are likely to use it as well, and that would still require said dependency.

minad commented 2 years ago

Suggestions welcome on how to retain it. It's a handy functionality, isn't it?

It is. I haven't thought much about it. Maybe we can come up with something. However it is also questionable if these many Company backends are willing to move over to some slightly modified calling convention.

Same goes for this. And since we're talking in the context of cape, I suppose you would want to provide the alternatives for those function in this package?

My proposal is to simply drop these functions without replacement. Completion backends are complicated enough. They don't win much by using the symbol grabber for the prefix. They can as well implement their own parsing. This is not particularly backend author friendly but note that the situation is not a bit worse than for usual Capfs and there are also functions like bounds-of-thing-at-point! I really consider these symbol grabbers a minor convenience which are not worth it, since they introduce this unnecessary coupling. Furthermore the grabbers could also not be made part of Cape since then the backend would have to depend on Cape which is undesired. The goal is to have company-* backends which depend on nothing, not Company, not Cape.

That aside, I was going to use company-grab-symbol to "transparently" migrate the return value of prefix from a string to a cons of numbers someday. Though maybe a suffix command would provide a better transition, given that prefix can already return a cons, and a list there would be kind of messy.

Oh this wouldn't work then.

Note that that adds a dependency on company, though. ;-(

Yes, but for the context of Cape I could just steal this function. It is not nice to have such a duplication, but the function is relatively isolated and it does not hurt if changes to it are merged back and forth between Cape and Company from time to time.

Yes: but it doesn't look nice when the full file name gets long enough to span the whole width of the window: company-mode/company-mode#1040

You can try Corfu, there I always show the popup only for the current field. With the Capf workaround it works even well in Eshell now!

I suppose a good test of this alternative would be to figure fixing this problem without bringing in the notion of "fields" from CAPF. Which I was kind of going to use, after I "figure it out properly".

Indeed. Ideally you can use the read-file-name-internal completion table directly or rather the cape-file capf (https://github.com/minad/cape/blob/700c9d7bc221e04e259947f8fb7a908bf1909bc0/cape.el#L416-L431).

Not really a problem for me, but I worry about backend developers feeling whiplash about the change in strategy (company? capf? company again?), and about being able to retain the niceties that the dependencies that you mentioned provide.

Yes, this is a reason to move to Acapfs. The niceties of the dependencies are not that important as I argued. Symbol grabber should go away and one has to find a better solution for the interactivity via company-backend-begin. Anyways I am just considering this possibility, not sure if it is more or less work or more or less realistic than introducing Acapfs.

company-grab-symbol could probably be replaced with bounds-of-things-at-point if we change/rename prefix into a method returning a start-end cons.

Yes. But you don't have to change the return value convention. Backend authors can apply their own normalization.

I suppose we could have a separate macro like company-define-backend-command, but then the backend authors are likely to use it as well, and that would still require said dependency.

No, this would go in exactly the wrong direction. We cannot introduce anything if we define an API in order to retain the decoupling. Backend authors should only use upstream functions.

minad commented 2 years ago

Without overthinking this too much, but the interactivity part of the backends could be retained easily:

(defun company-css (command &optional arg &rest ignored)
  (interactive (list 'interactive))
  (cl-case command
    (interactive
     (when (fboundp 'company-begin-backend)
       (company-begin-backend 'company-css)))
    ...))

(defun company-css (command &optional arg &rest ignored)
  (interactive (list 'interactive))
  (cl-case command
    (interactive
     (when (boundp 'company-interactive-function)
       (funcall company-interactive-function 'company-css)))
    ...))

(defun company-css (command &optional arg &rest ignored)
  (interactive (list 'interactive))
  (cl-case command
    (interactive
     (funcall (symbol-value 'company-interactive-function) 'company-css))
    ...))

Different frontends could just set different company-interactive-functions. It does not look pretty, but for my taste it would still be acceptable if as a result I get a frontend agnostic backend.

dgutov commented 2 years ago

It is. I haven't thought much about it. Maybe we can come up with something.

Perhaps Company could also provide a completion-in-region-function (is it theoretically compatible?). And could be activated through it in a general way.

However it is also questionable if these many Company backends are willing to move over to some slightly modified calling convention.

The most popular ones are built-in. You're going to have another problem with them: they're simply distributed together with Company, it's an implicit dependency.

As for the rest, you can start writing some. Or other people. I guess the important part would be championing a new interface that's independent from Company, and to choosing a name which is something else than "Company backends" (right?). The fact that existing Company backends will satisfy that interface will be a feather in the hat of this initiative.

My proposal is to simply drop these functions without replacement. Completion backends are complicated enough. They don't win much by using the symbol grabber for the prefix. They can as well implement their own parsing.

If we postpone that until after Company backends' migration off prefix, it should be less of a problem.

I really consider these symbol grabbers a minor convenience which are not worth it, since they introduce this unnecessary coupling. Furthermore the grabbers could also not be made part of Cape since then the backend would have to depend on Cape which is undesired. The goal is to have company-* backends which depend on nothing, not Company, not Cape.

Perhaps, but I don't know how much of an advantage that low barrier of entry really is. When you can create a backend from scratch very easily, and move on to the "meat" of the task, it's really encouraging. So I think that's added to the popularity. Though of course there have been other factors, which might have been more important.

Yes, but for the context of Cape I could just steal this function. It is not nice to have such a duplication, but the function is relatively isolated and it does not hurt if changes to it are merged back and forth between Cape and Company from time to time.

If you're willing to copy/sync the subsequent changes. A number of outstanding issues should require corresponding changes to that function. I will hopefully get around to those soon-ish.

You can try Corfu, there I always show the popup only for the current field. With the Capf workaround it works even well in Eshell now!

The question I'll like to answer is whether the notion of "fields" is essential for good UI experience, or whether it can be substituted entirely with prefix/suffix. ATM I'm thinking it might be necessary to avoid finishing completion when the user chooses a directory, to let them pick an element inside that directory, etc (keep showing the popup, but now with the list of that directory's contents). But then one would need to do something else special to only input that directory and not one of its children.

dgutov commented 2 years ago

Different frontends could just set different company-interactive-functions. It does not look pretty, but for my taste it would still be acceptable if as a result I get a frontend agnostic backend.

If we have a company-* variable, where would it be defined? Sounds like a dependency on Company still.

But perhaps it (and multi-adapter code) could merge to a minimal support package that would be a dependency of Company as well.

minad commented 2 years ago

Perhaps Company could also provide a completion-in-region-function (is it theoretically compatible?). And could be activated through it in a general way.

Yes, that would be good! It should be possible to create this.

The most popular ones are built-in. You're going to have another problem with them: they're simply distributed together with Company, it's an implicit dependency.

Okay, but the ones built into company are not that important for Corfu/Cape. I already have the most important backends replicated here in Cape. I am more interested in third-party company packages or company integrations which were out of reach of Corfu before I added cape-company-to-capf.

As for the rest, you can start writing some. Or other people. I guess the important part would be championing a new interface that's independent from Company, and to choosing a name which is something else than "Company backends" (right?). The fact that existing Company backends will satisfy that interface will be a feather in the hat of this initiative.

Sure, but then I could just implement Capf backends right away. My idea was rather to convince authors of company-* to move over to a more reusable API.

Perhaps, but I don't know how much of an advantage that low barrier of entry really is. When you can create a backend from scratch very easily, and move on to the "meat" of the task, it's really encouraging. So I think that's added to the popularity. Though of course there have been other factors, which might have been more important.

I agree fully with you that a low barrier of entry is good. And company backends are indeed easier to write thanks to the different actions instead of the metadata extension functions. But this symbol grabber seems still like a very minor convenience. From my perspective this minor convenience does not justify incurring a Company dependency, while the API is already almost perfectly independent of Company.

If you're willing to copy/sync the subsequent changes. A number of outstanding issues should require corresponding changes to that function. I will hopefully get around to those soon-ish.

Sure, changes should be synced in both directions.

The question I'll like to answer is whether the notion of "fields" is essential for good UI experience, or whether it can be substituted entirely with prefix/suffix. ATM I'm thinking it might be necessary to avoid finishing completion when the user chooses a directory, to let them pick an element inside that directory, etc (keep showing the popup, but now with the list of that directory's contents). But then one would need to do something else special to only input that directory and not one of its children.

You should really try corfu-mode once in an eshell buffer or try cape-file. I like this completion experience where every field is completed separately. But maybe it is a matter of preference. Try mkdir some-pathTAB and cd some-pathTAB.

If we have a company-* variable, where would it be defined? Sounds like a dependency on Company still.

Yes, my proposal is not very clean. The idea would be that the variable can be set in the user config and in the company package. Since the check is guarded by boundp or fboundp the company-backend package wouldn't have to depend on Company. For Corfu/Cape users, the function variable could then be set accordingly in the user config.

But perhaps it (and multi-adapter code) could merge to a minimal support package that would be a dependency of Company as well.

Hmm, yes. Maybe this would be an improvement if there is a small company-api package which provides only this support code. However I am usually not fond of excessively splitting up packages (even if it seems so with my packages ;). Usually I try to find clear boundaries. In this case we are not far away from not requiring any support package at all. One can just write company backends without a dependency. I think that should be the goal if one is designing an agnostic API.

See the example Company backend in the Cape README: https://github.com/minad/cape#company-adapter. Ideally all company third party package would follow such a style - no Company dependency, the API and the backend are purely based on Emacs built-in facilities, which makes them completely agnostic.