Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Use of company-capf backend in eglot-managed buffers
* eglot.el (company-backends): forward-declare (eglot--managed-mode): Force company-backends to company-capf
- Loading branch information
a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? What if one want to use several company backends? This overrides any customization made to
company-backends
.a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's simpler. It's the same with flymake, really. In clangd, for example, the default company-clang backend interferes with Eglot's.
I know what it does. My reasoning is that if you're going to let Eglot "manage" your buffer, you let it manage your buffer. You get your old backends untouched when you disable eglot--managed-mode.
What exactly is your use case for non-eglot-completion in eglot-managed buffers?
a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depending on your answer, I can either completely roll this back or find some middle-of-the-road solution.
a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In case I use eglot for clang then I will remove company-clang from company-backends.
There are so many ways to configure variables for user needs: customize, mode hook, dir-locals. Why override? C'mon, is this lsp-mode? :)
My company-backends:
(company-capf company-files (company-dabbrev-code company-dabbrev company-keywords))
, plus maybecompany-yasnippet
.a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is unnecessarily complicated. I will ask in the company-mode repo if they can switch the order.
lol, ok
And do they all work together?
Remember you already have a lot of this power in
completion-at-point-functions
, whichcompany-capf
could/should leverage. Ideally, we want to stay with Emacs's core idioms for doing things, and avoid repeating ourselves in extra packages.a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I may chime in here, I think I still want other company-backends when I'm completing in a string or comment. I don't think LSP servers will return completions in that case; I'd hope eglot would return nil to pass on to the next company backend. Similarly if I'm typing a function name for the first time in this project, but I've used that name before in other projects, some other dabbrev-style backend may know about it. (
company-yasnippet
is another case, but that might get really complicated -- I could actually see a use for grouping that withcompany-capf
incompany-backends
but I haven't used yasnippet enough to really know.)a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I'll roll it back and find some other solution.
a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, though
company-files
stands in the way sometimes. Also I have packagecompany-try-hard
which allows to see candidates from other backends.I'm for it with both hands. My point is that
company
configuration is up to user. There are no eglot related code in this commit ;)a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤦♂ I personally need
company-facepalm-very-hard
right now. Sure you wouldn't rather turn your (deserved, admittedly) rage towards this commit into an energy boost to work on good backends forcompletion-at-point-functions
?a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry if I sounded angry. Rage wasn't meant at all. I just wanted to know your motivation on this commit.
Even if it stay like this, I can configure company in egot mode hook or something.
What backend do you mean? Like
yasnippet-at-point
?And thanks for your work!
a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, "rage" wasn't the best choice of words. "indignation" would have been more appropriate. And I hope it came across as a joke, which it was.
Yeah, yasnippet-at-point, file-name-at-point, whatever-at-point
Yes you can. Well, almost, you have
eglot--managed-mode-hook
but that is an internal var. But you can use it. I've been meaning to make aeglot-managed-mode-hook
instead. I could also make a new variableeglot-dont-trample
that you could add the symbol:company
to, and then eglot would stay out of your config.a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could just push
company-capf
onto the front ofcompany-backends
if you want it to be considered first, right? (No need to remove the user's other backends.) That would be what I'd do anyway, as a company user, if I know that eglot usescompany-capf
(well,completion-at-point-functions
) for its completions.Your larger point is worth thinking about though -- as
completion-at-point-functions
has gotten more capable, why do we really need company? It's just that everyone uses it, and company undeniably has momentum.a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, and let me just apologize to @dgutov for dragging him here for the millionth time:
To be clear:
c-a-p-f
. But I hope that can change.a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I was going to ask Dmitry to do that, but maybe I can do it in eglot myself.
a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wish we could say that, but people do use its backend grouping feature, and there's no corresponding feature in
completion-at-point-functions
.EDIT: Apparently you all know that, I just responded before reading the thread.
a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? Is that because
company-clang
might take priority sometimes?a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. The clangd developers (who BTW are cool enough to list Eglot as their favoured Emacs alternative) even have this pitfall listed https://clang.llvm.org/extra/clangd/Installation.html
a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. So there's one problem with that:
completion-at-point-functions
has a default value, and it polls the current tags table if one is loaded.So if somebody has a habit to use
company-clang
while a tags table is loaded, it will stop working by default. And the two can help each other: Clang for completion, tags for navigation.Is
company-clang
the only problematic backend for you?a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've read
company-backends
already, but I don't understand how wonderful that is. If it indeed has merit, we can add toc-a-p-f
.Anyway, here's a bold idea. We move
company.el
to Emacs core (Eli permitting, of course, :-) ) and dillutecompany-capf.el
in it. Then GNU ELPA'scompany
package can become just the backend management code and the familiar default value ofcompany-backends
with all the bells and whistles for Company junkies.Apart from some namespacing and versioning gymnastics (the
:core
ELPA pacakges will come in handy), I think this is doable.a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another hint that having two backend thingies sucks.
You lost me there, but I believe you. Can't we
a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
heh. Can't we fix it? :-)
a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
People usually combine company-dabbrev-code, company-yasnippet and some semi-smart backend like company-etags to merge their completions. It's better than nothing when there's no smart enough completion backend (that does all these things together), or when the said smart backend is misbehaving.
a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please be my guest. I thought about it, and didn't even decide on an appropriate API.
Overall, minibuffer.el looks rather (over-)complicated in certain places to me, that's one of the reasons I'm not in a hurry to do more work in that area.
a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hope you can see that as proposed, it'll only add up to maintenance effort required on my part. So I'm not in a hurry, really. And I probably repeat myself here.
In any case, I don't think this can help you avoid dealing with both UIs, with both of their peculiarities. The
M-x completion-at-point
UI is well-established, has its userbase, and even has extra features that are non-trivial to implement with a popup-based UI. Or, at least, might look weird.a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But it's working as intended. Isn't it? Company defaults to
company-clang
as soon as it sees an opportunity to do so (functioningclang
executable), and having tags-based completion is certainly better than nothing.I think any change in that area will likely break somebody's workflow. Not to say we must never do it, but I wouldn't call it a "bugfix".
a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't take this approach: "no integration whatsoever untill full integration". We should take small integration steps like the one I proposed earlier.
I don't understand why
I suspect that once company.el is in core, we'll possibly see a great decrease in downloads of the ELPA
company
package, less bug reports to github, etc. Your work there would be greastly diminished. And in Emacs core it would be shared with the other developers. Not to mention a lot of integration problems are instantly gone.Yeah some things grow so large, maybe sometimes slightly monstruous, that they start to present these roadblocks and then nothing gets done. The alternative I sometimes pick is a rewrite, so would you oppose I write
proletaire.el
orfreelance.el
(you know, as in, an alternative to "company") based off thecompany.el
frontend? Then one day company can useproletaire.el
, who knows? Maybe it's even easier if some parts of it are written in C directly.But I still think merging
company.el
is slightly easier: it has none of the copyright problems of Flycheck and a decently fitting design (except, as we are seeing, those creative backend gymnastics).a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just outlined slightly different steps, is all.
And more dealing with said reports through debbugs? Oh joy.
shrug, I don't know. Still waiting for prolific contributions from others to xref.el and project.el.
Will they be gone? I suspect that would depend on the details of the integration.
The design of company-backends is pretty clean. Cleaner that the design of completion-at-point-functions and completion tables combined, that's for sure.
Of course not. But I suppose you would want to start with extracting the popup frontend code into some reusable library. It's been on my list for a while. 😔
See my notes above about two different UIs, and about completion-at-point having a different, somewhat incompatible feature set.
a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So IIUC, is the first step enhancing
c-a-p-f
to work with grouped backends?lol
True, true. Same here, of course. But still, it a least comes with a good deal of scrutiny. It's harder to make a momentous design error with so many watchful eyes.
The
company-capf.el
double-indirection thing will mostly be dilluted away, I think.This is not saying much :-) A design based on generic functions would probably be much stronger. But I'm not proposing that (yet). I say, integrate a
company-core.el
, including just the most basic frontend andcompany-capf
. Make it a:core
package. Then give all the Company junkies their stuff in ELPA/MELPA under the same package. I did this withjsonrpc.el
and it worked nicely.Actually, what happens is that you get most of your bug reports through Github as always, but you happen to fix them in Emacs core (and increase the version tag of
company-core.el
so ELPA rebuilds it, which gives you at most a one-day delay).Sounds like a plan, actually. Provided I won't be dogged by many compliance problems, I can do that. Any pointers? What is the "popup frontend code"? Is it similar to what
autocomplete
used in https://github.com/auto-complete/popup-el? BTW do you have any idea what the copyright assignment situation in autocomplete is?a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think so. Preferably, while retaining the transparency of its values (i.e. a grouped backend is a list, not a opaque closure).
I've honestly seen very little help in both. Lots of discussions and opinions, yes, but very little proposals that keep the existing design goals in mind. Maybe it's a fault of mine for not documenting them enough, but it'll likely be the same with Company.
I don't think so. How else the external "full version" package would work, if not using
company-backends
?You can understand if I'm not in a hurry to convert the existing backends before that. And we do need them, I think. At least the -dabbrev pair. Maybe -keywords and -files as well.
I guess. I'm still not seeing where the reduction in complexity will come from. All the above will increase it, though hopefully not too much. And the sole benefit is that (a version of) Company will come with Emacs by default. Which is not too bad marketing-wise, but only that.
And without a posframe-based frontend in the bundle, it will continue to be slightly-awkward.
The
company-pseudo-tooltip
code.You can try to mimic the API of
popup.el
, but I'm not sure. Due to implementation details, it might not be the best fit.I asked if they wanted to contribute the code (in popul.el), and got a refusal. In one of the issues there.
a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tell me about it... We're just wrapping up a giant thread that went nowwhere, when an easy obvious solution was in plain sight... Well there are good and bad days.
"them" who? the backends? Are these special company backends?
If you don't give up any functionality in the name of integration, then no reduction in complexity, no. Only increase, indeed as you point out. It seems reasonable, though, that once Emacs has a core company-like completion frontend, company will drop
company-capf
in favor of it, right? And maybe, even if it's momentarily inferior, people will start coding forc-a-p-f
, so you can deprecate some of your stuff. But if you don't give up anything...If
popup.el
is out of reach copyright-wise, then I might as well write the right API based on company's needs. Any ideas?So a "rewrite" roadmap: extract a
popup.el
-like bit from company. Extract a couple of company "frontends", maybe the two most useful, or maybe just the one with the dropdown box intoddown.el
. Put them in:core
packages, make Company rely on these. Re-evaluate the situation.Uhh, interesting, what is that? is it a proper dropdown box without overlay hacks?
a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right.
Just backends. Pretty basic ones.
But you yourself said that there would be a third-party package for "enthusiasts" or whatever. And it will need a plug to plug into.
If you have a plan to reduce complexity, then we can discuss it. For the time being, we can drop the "moving to the core" step, because by itself, as you can see, it doesn't make anything simpler.
You can look at what the popup code calls. :-)
Though ideally, we should look at the other packages, ones using popup.el. We'd really want a replacement for it in GNU ELPA, if at all feasible.
Or just put them in GNU ELPA, because at that point there will be no consumers of said libraries in the core.
Proper-ish, yes. I don't use it myself (yet), so you can be the judge of it yourself.
https://github.com/tumashu/company-posframe
a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, there will. This is the "slow rewrite" route, remember? If I extract those two libraries so that I can make a much lighter
completion-in-region-function
based on them. So that Eglot can rely on that by default, instead of Company. Same thing with asnippet.el
I have stashed somewhere gathering dust, but still functional.a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, okay. It's as good approach as any to have popup-based completion UI in Emacs.
I wonder if it ends with you unsupporting Company, though, or something like that.
a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see no reason not to keep supporting
completion-at-point
and the:company-*
cookies, which, for all I know can become standard emacs symbols...a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, if that was enough for everybody to be happy, we wouldn't be having this conversation, would we?
And I seem to recall a problem caused by caching that relied on some extra assumptions.
In any case, please go ahead. I certainly don't want to discourage you from working on a popup.el alternative.
a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What "was enough"? I don't follow... Do you mean
company-capf
is not enough for everybody? Probably not, but it's probably enough for the overwhelming majority. It's that 80-20 rule, 80 percent of the features you only use 20% of the time, but here it's like more 95-5.That said, what can't
company-capf
do?I believe it was settled, but I can't be 100% sure. I also can't be sure about what the assumption was... Was it "assume that the
c-a-p-f
element is only called once per completion attempt?". I'm relying on that, I think, and it seems to hold for Company and barec-a-p
I'm going to try. In any case, I must admit I'm pretty pleased with my "
company-capf
only by default" choice.a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like, Company, the way it is now. Something is motivating you to do the conversion work, and it's not just having popup-based completion in Emacs by default. Right?
Element meaning function like
elisp-completion-completion-at-point
? Well, not necessarily.company-sort-by-occurrence
, when used, will call it on multiple positions in the current buffer, though discarding the completion table returned. Maybe there are other examples, but in general your expectation is probably close to the truth.a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, it kind is. That's what I want, yes. Unless I'm missing something in my own reasoning :-). What did you think it was?
Basically company does too much for me, it does a superset of what I want. It's the best we have, tho, so I have to take all of it. It's very much like yasnippet: it's got a lot of 2008-naive-joão-cruft built-in that I wish I could separate. But it's really hard.
Yes, sorry. I don't know why I even wrote "element".
What's
company-sort-by-occurence
? Can it override the explicit request from thecapf
function to be sorted in a certain way?Anyway, if it discards the completion table everything's fine.
let's hope so
a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For instance, working around having to deal
company-capf
's possible uses in groupings, for example. Or even some implementation decisions incompany-capf
.There is some, but aside from older backends, I wouldn't say there's a lot of stuff that is siginificantly outdated. Though, of course, it's the prerogative of the "next generation" to try a rewrite to "simplify things". :-)
Yes. If the user adds it to
company-transformers
.a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do believe some of my work can lead to simplifying Company's implementation, notably if I write libs that company can depend on.
But Company or any other lib is not a goal in itself. A good user-experience out of the box is. I know Emacs lets you customize everything and there many users are proud of their configuration achievements. But for each one of those many more really don't care. Where I work, people just want Emacs to work, they take what the person next to them has, and sometimes it is a keybinding multi-library multi-snippet monstrosity. When they ask me to work on their computers, it's a real pain.
And just thinking about learning what a Company grouping is makes my head hurt.
Anyway, I'm ranting here. But I would be curious to see your
.emacs
. Is it very complicated, do you use many Company features? I would guess not, maybe I'm mistaken.Fingers crossed they don't, or that it doesn't break the assumption I had before.
a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Emacs should really have better defaults. And the packages as well, everything. So that more people are comfortable with using said defaults.
That's not to say that our users are going to be content with little customizability.
shrug It's a popular feature, we've mentioned it several times already in these discussions. Most of what you need to know is under the heading "Grouped backends" in the
company-backends
docstring.Frankly speaking, it usually serves to make up for the absence of a sufficiently smart backend for a given language/environment. One that offers classes and methods, and local variables, and useful snippets, all in one package. The situation is gradually improving, but I don't think this use case is going to go away anytime soon.
It's, um, here: https://github.com/dgutov/dot-emacs. This version is hugely outdated, but the many small changes I've done since don't really change Company configuration. I'm not sure you can gauge my usage habits from it, however.
I don't use the inline search, or, usually, any of the async backends. The rest comes into play one way or another, I think. At least when we're talking about major features.
I'm saying I'm concerned about frontend independence, is all. As we have established in the nearby discussion, Emacs offers lots of knobs and toggles, and one can get away with breaking the contract if the post popular completion frontend doesn't mind. As such, if we didn't stumble upon
company-require-match
, the behavior of returning nil arbitrarily would still be there.BTW, I hope can we both agree that while the intent behind
cancel-on-input
there is good, it really should use a different API where the request returns some kind of cancelable promise, and where the frontend cancels it upon detecting user input. That's why I'm looking at the API as the main part that needs improvement.(I'm using "frontend" here to mean any kind of system interacting with c-a-p-f. So in practice either
completion-at-point
or Company.)a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very often a non-starter, because there are many, who, legitimately, think the current defaults are fine. New defaults have to be, as much as possible, keystroke-compatible with previous behaviour. So arguing
electric-pair-mode
makes sense, while arguingido-mode
doesn't (buticomplete-mode
probably does).So it's better that new behaviour should be very easy to add, at most one
M-x
away.M-x eglot
,M-x auto-tooltip-completion-mode
.It's "popular" amongst the people that let you know that they like it. IOW, its relatively popular. But you should admit the possibility that a very big part of the rest of the Emacs world doesn't care.
And the "Grouped backends" section is certainly not enough to understand what the benefits of it are. It points to the rest of the docstring, to the relatively cryptic commands that each backend responds to. An example is needed.
But I can more or less speculate what it's about and I'm not saying it's never going to pop up in
c-a-p-f
functions. It probably will, once people actually sit down and invest inc-a-p-f
backends. When we come to that bridge we'll see what can be done about it.I don't understand the rest of your reasoning, but I don't oppose that, I think.
And probably hurting nobody.
Maybe, though in the end it's always going to be based on the cancel-on-input argument. You can make
dgutov/promising-request
, if you want, on top of it. I considered it, but the abstraction's weight surpassed the benefits.a11a41b
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shrug I've been using Ido for years, and still do, but I wouldn't say it's optimal as a GUI anyway.
Sounds good, of course. As long as it's also easy to replace and extend. And doesn't diminish the possible power of said replacements.
If you are commenting on the docstring being insufficient by itself, okay. Help with documentation is always welcome.
Yeah, okay. I still believe that c-a-p-f will likely need to look a lot different for this to happen.
Rarther, I think it will use
jsonrpc-async-request
and a new functionjsonrpc-abort
similar torequest-abort
in request.el.