Idea going forward #802
Replies: 10 comments 3 replies
-
Hi @theothornhill, thanks for creating this discussion. You're right that Eglot has fundamentally two jobs, and this is not always very clear: a) Create, via b) Provide a programmable API to access LSP functionality so that other interfaces to LSP features can be built. Job Job
I think an Not sure if should be a generic function though. Maybe it should, maybe it shouldn't. To help decide these things, I normally type out three or four examples of uses of that function. Just one is usually not enough. We need to see how often the new helper matches a given use pattern. I also sometimes use the LOC test. I use LOC as a proxy approximation for complexity. In a refactoring, if you can make the overall product do the same or more with less LOC, excluding the new helper's, it's a good/decent change. If you can make it with less LOC including the helper's it's a GREAT change. If you can't make either, it's probably not worth it. Right now, LSP rename is 99% tucked in nicely into a single function,
But who is this user? In your |
Beta Was this translation helpful? Give feedback.
-
Yes, absolutely. I think making this distinction clear will be a huge win for eglot. Also that would make
The problem is that they do exist. Take F# mode, for instance. They have created this: https://github.com/fsharp/emacs-fsharp-mode/blob/master/eglot-fsharp.el. This in turn is a problem, because IMO F# isn't really that good, and development over there is very slow. This lead me into having to make my own, simpler F# mode, To make things work properly in my work setup, I have to (admittedly a hack): (cl-defmethod elsp-handle-notification
(_server (_method (eql :fsharp/notifyWorkspace)) &key payload &allow-other-keys)
"Handle notification window/showMessage"
(elsp-dbind ((FsharpWorkspacePeek) content) payload
(elsp-dbind ((FsharpData) Data) (json-parse-string content :object-type 'plist)
(elsp-dbind ((FsharpFound) Found) Data
(elsp-dbind ((FsharpData) Data) (aref Found 1)
(elsp-dbind ((FsharpFsprojs) Fsprojs) Data
(vconcat (seq-map (lambda (x) (list :uri x)) Fsprojs)))))))) Notice that here I think these things should be possible to do with less friction. If one major mode is slow to respond to development, we should be able to devise our own hacks to make things work. In addition, some servers provide an enormous amount of extensions, such as java and omnisharp. All of them cannot be reasonably implemented quickly in the first iteration of this, but they should be invited to be made. And there should be an extra :core package in elpa,
I think they should be, because that way they are automatically hooked in by (cl-defmethod elsp-handle-notification
(server (_method (eql textDocument/publishDiagnostics)) &key uri diagnostics
&allow-other-keys) ; FIXME: doesn't respect `elsp-strict-mode'
"Handle notification publishDiagnostics"
(when (elsp--diagnostics server)
(elsp--update-diags server uri diagnostics)
(run-hook-with-args 'elsp-after-diagnostics-hook server uri)))
Yes, it isn't bad, but it can be better.
Yes, I didn't make an implementation there. But to make a point of what could be possible to do there:
The point is that these things should be enabled, not seen as details for some future patch that as to be agreed upon by everyone involved |
Beta Was this translation helpful? Give feedback.
-
TL;DR: I think these things will appear definitely clearer to all users once Eglot is in core. That's were most of the consumers are and even if they're not (like F# mode), it's where it's much easier to look, primarily.
In what way? In the way you are describing it, it is very clearly made worse in that regard. But making a single function worse "locally" is OK as long as that brings some advantage somewhere else so that "globally" things are clearly better. IMO, this is what refactoring is about many times.
OK, but I think some of these things can be contributed to Right now, as it stands, I don't think this is an use case to promote
OK, as I said, I need to see more cases. Just 1 isn't enought. It needs to be 4 or 5 and ideally pass the LOC test. Also I don't know how familiar you are with generic functions (or Common Lisp where they come from), but IMO you are making poor use of generic functions. Your example:
Here, I think it would be better to use a
Of course you would need a The set of generic functions that enable a given API is called "the protocol" in Common Lisp parlance. In my experience, protocol design is hard, and should be iterated. Here's an example of another protocol, with a regular function and two generic ones:
Or maybe Or, if you're feeling extremely ambitious, you can build an actual hiearchy of CLOS objects that represent the LSP methods are then express the hierarchic relations between them ( So, in summary, there are many possible protocols and finding the right one takes time, experience and more API use cases.
I want to understand this F# example and exactly the "friction" you're referring to. Are you referring to the need of chaining many
|
Beta Was this translation helpful? Give feedback.
-
Well, to me LOC is not at all one of the more important metrics to go for. I'd rather say that a clean, understandable api, with clear domain boundaries are to be preferred over LOC. In this regard my experiment is better. The important thing here is to consider how we'd want an optimal lsp implementation in emacs 29 to look, not how we can make eglot a little better. We agree fully on handing responsibility over to their separate libraries in emacs, and you are way more experienced and knowledgeable than me in this regard. However, I still think that learning the ins and outs of eglot has been a little difficult, and I believe this difficultly may translate over to other people trying to extend it.
Yes, some of it should be implemented in eglot. Some of it should absolutely not. And this freedom should be imo be considered when designing the internal apis.
That's right. I didn't do it, and i'im still not sure how I'd do it. That's not the main point. I'm trying to show the separation of domains in eglot, and trying to start a discussion for where and how such a division should be made. My proposal is pretty clear, I hope.
Yes, I agree.
Yes, making such a protocol is hard, and yes, iteration is important. Consider elsp iteration 0.5 of eglot, and let's try to settle on a proper protocol when emacs 29 lands. This might require discussions and refactorings, but I hope it'll be worth it in the end.
Yeah, I considered that when first forking, but I found it a little cumbersome. I actually started with a hash-map of handler-name->handler-function, but realised that the generic functions are a kind of hash-map, or at least a namespacing, so I went with a very simple and naive protocol.
Agreed. And I want to help with this, if needed.
By friction, I mean that emacs packages, as all open source projects are subject to the maintainters time/energy/willingness to imlement the feature In
It seems you wanted to write something more here |
Beta Was this translation helpful? Give feedback.
-
Every single developer I have ever known has said exactly the same in one form or another. But of course you know that this means different things to different people, with different usages, styles, ages, patterns, backgrounds etc. I.e. it's super-subjective I use LOC as a reasonably objective proxy for complexity which is what one want less of. Complexity is a more objective thing, not such an elusive thing. One wants less repetition, too. Repetition is much more objective. One want localily, the ability to reason locally about a piece of code without inquiring about other pieces of code in other places. For example, your suggestion destroys some locality to I'm not saying there is not merit in these proposals, but they have to come with real-world use cases, and in that discussion the use cases one presents are also subject to discussion. Again, I think the vast majority of future LSP users live in Emacs right now and are maintained by a bunch of people that should definitely be involved in this discussion. They are the consumers of the thing we're going to design so doing this with a very faint, if any idea, of what they need is misguided to say the least. IOW, we should not start throwing solutions, lovely as they might seem to us, at a problem we don't understand completely.
Not "maybe" for me. A full on "yes". Idiosyncrasies shouldn't be "covered", they should be solved at the source. Eglot is not in idiosyncrasy-covering business. Quite the contrary, Eglot is a project designed to promote solutions to technical problems and good design in the IDE business, even if that takes time. If in the meantime it is useful to users (it seems that it is moderately so), then all the better. Some other project may -- legitimately -- be aimed differently, So here, in particular, F# mode should be brought into Emacs, so that you, who has the willpower and knowledge to fix it, can fix it. Is it not free software? Then make a free software version of it, based on good Emacs tools. You'll probably make a better F#-mode in the process.
Ah, I can't remember what it was anymore! |
Beta Was this translation helpful? Give feedback.
-
Yes, I know this. And this is also why I try not to marry my own proposal, and I see many things wrong with the current
Yeah, I think complexity is almost as elusive as all the other things we have discussed throughout. Repetition I'm on board with. But the reason I didn't yet address the repetition in my code was that I, same as you, like to see how the code evolves, and try not to move things around too much until it's too painful not to. As for the locality in https://git.sr.ht/~theo/elsp/tree/master/item/elsp.el#L2206 The advantage in my implementation here is that for a new reader of this code, such a line would signal a "hmm, this is something I've seen before", whereas in eglot it's just another 5 lines of very complicated lisp in a very long function. This is a tangible benefit. Whether or not it outweighs other pros and cons it less clear to me.
I believe I have presented some tangible benefits, but I'm convinced you don't see it that way.
I'm not sure I understand what you mean by this sentence. But yes, the more the merrier.
Well, I'm a user, not the maintainer or a collaborator as far as I can tell, and I try to make useful changes when eglot falls too short in my dayjob. I also try to avoid making choices in the implementations I provide for this exact reason. In addition, this proposal is made exactly as a way for emacs not to make too many decisions on the end user experience.
Covered was the wrong term. Solved is better, yes. However, the lsp spec is a moving target, and lsp extensions is a real thing. This makes the train go even faster. If eglot is not capable of implementing or at least hacking in
Some of them are lazy hacks, but some are actual, useful functions. For example,
It is free software, but ime getting things into emacs is a long process, at least when the meritocracy hasn't fallen in their favour, even for the ones as stubborn as me.
Also, as a side note: when playing with |
Beta Was this translation helpful? Give feedback.
-
And you are. I'm sorry if I was not. I did not mean to be harsh. You do great work, I'm jsut trying to state my point of view of what I think is desirable and why. I have not yet understood your proposal as a whole, I really haven't. I wanted to explain how I think .
Yes,
But are you sure it can't? Seems what
Yes, I've seen this (I've also seen the inverse movement). I don't care much to be honest. I care for users to explain their use cases, evaluate it, and then care for users that present solutions to those use cases. If the solution is mediocre (in my view), I'll try to evolve eit with the user to make it better (again, in my view). If we can't reach an agreement, that's fine. I'd rather lose that user, suggest that he forks. That's because I think a mediocre solution makes it harder to eventually search for a good one. Also I'm not in this for the popularity contest :-)
I meant that the majority of users of the API you're envisioning/putting, which I assume will be major-mode maintainers, are not in this discussion forum. And that this discussion is something to be had once So I don't really want to commit to anything prematurely.
I don't see it that way indeed, but I don't understand your proposal yet. But even if I did, I would find it hard to evaluate the benefits' tangibility, becasue as I said, there are too many unknowns.
Yes, I noticed that too! That's good :-)
Yes, you're totally right. I misexpressed myself. |
Beta Was this translation helpful? Give feedback.
-
Hey no worries. I think I see where some of our miscommunication stems from. In this issue (#802), The point is not to make a case for why elsp is better than eglot, as it is not. It is different in some respects, but mostly it is the same. However, when playing with elsp I noticed that the distinction you also made, with the
Yes, the parts I've removed are the company-parts, and should of course be present, but wasn't relevant for me in the lsp context at the time. My biggest problem with that function is that it's control flow is pretty hard to parse. Some functions are let bound, some of it uses helpers, and the cache etc etc, To me this is a case where breaking a little locality may be desirable, but then again, maybe not :)
You're not missing anything, but seeing how this has not yet happened, led be to the elsp experiment. I don't think it is clear for anyone that this or that should be in eglot and other things should be in major modes. I am convinced there are improvements to be made that will help with this, though I won't really go further than that for now, I think.
Right, I understand now. This is also one part where I think we have miscommunicated, or I have been unclear. Major mode authors are one of many groups of consumers of the eglot API, in my opinion. One other group I'd like to make a case for is the regular user. For example, consider the code action functionality in eglot. One of my first contributions to eglot was to remove
Yes of course. There are many people on emacs-devel that surely should weigh in with way more weight, for obvious reasons. I'm sure SM has lots of good ideas. However, as tree-sitter is orthogonal to lsp I don't think it would be relevant. Still it'd be nice to have.
Hopefully this post has made my stance a little clearer.
Hehe, I didn't mean it that way, nor take offense on others behalf. What I meant is that many servers offers extensions to the protocol that needs handlers to be made specifically for that language. There is no generic BTW. There are other, easier to contribute parts of elsp that should be turned into prs. Eglot does the wrong thing in many places. For example there are several cases where we send a request, but it should be a notification. For example, the beautiful ( :) ) :before hack on should be a :before on notify, IIRC. I'll compile a list of these things sooner or later, if you'd like that! |
Beta Was this translation helpful? Give feedback.
-
Theodor Thornhill ***@***.***> writes:
and make that distinction clearer. I think I've made a little progress
on that, by using the backstage/on-stage analogy (I figured you'd
catch on to that, as it's from AMOP).
Heh, no I haven't. I have not read that book entirely, just consulted
it (often)
So just to be clear. I don't want to make a super pr changing eglot
into something other than what it is just for the sake of it. That
serves no purpose. However, the discussion for how to best enable
consumers of eglot per emacs 29 is surely good to have. I don't
consider myself an authority in any of the areas eglot touches, but I
do like what it is.
I think soon a change developing what you and I have agreed that is an
important part of Eglot "going forward" is going to happen. In Eglot, I
put some of the APIish in
;;; API (WORK-IN-PROGRESS!)
And a very much a work-in-progress it is.
> I meant that the majority of users of the API you're
> envisioning/putting, which I assume will be major-mode maintainers,
> are not in this discussion forum. And that this discussion is
> something to be had once eglot.el in its current form lives inside
> Emacs
By the way, I meant "putting forth" there. That's what you get when
kids are running around asking all sort of questions and you're trying
to write a technical email.
Right, I understand now. This is also one part where I think we have
miscommunicated, or I have been unclear. Major mode authors are one of
many groups of consumers of the eglot API, in my opinion. One other
group I'd like to make a case for is the regular user. For example,
consider the code action functionality in eglot. One of my first
contributions to eglot
Yeah, it's nice to have users make Eglot what they want, too. But I see
them changing the part a) "Eglot as a UI/minor mode". The things one
does with the API are either things 1. to contribute to Eglot itself
2. to contribute to major modes making use of Eglot's LSP lib. But they
can live in user's .emacs for a while, sure.
Yes of course. There are many people on emacs-devel that surely should
weigh in with way more weight, for obvious reasons. I'm sure SM has
lots of good ideas. However, as tree-sitter is orthogonal to lsp I
don't think it would be relevant. Still it'd be nice to have.
Yes, it's orthogonal, but it will, _along_ with LSP, reshape how new (or
existing) major modes are done (or redone) in Emacs. And I thing it
should be cohesive.
BTW. There are other, easier to contribute parts of elsp that should
be turned into prs. Eglot does the wrong thing in many places. For
example there are several cases where we send a request, but it should
be a notification. For example, the beautiful ( :) ) :before hack on
should be a :before on notify, IIRC.
yes, there's a good case for `eglot-request`, for example. If you look
deep down in the history of Eglot, you'll find eglot-request :-) I took
it out because it didn't do anything valuable. But you've found two
potentially interesting things already:
1. send the didChange thing (if the doc changed), and remove the
beautiful hack
2. check the associated capability or bork. Would be nice if LSP method
name and capability names could be mapped directly, or at least not
very verbosely.
I think this could make a nice PR. That doesn't add many lines ;-)
I'll compile a list of these things sooner or
later, if you'd like that!
Yes, if you've dived into eglot.el's code internals it's very useful
stuff.
|
Beta Was this translation helpful? Give feedback.
-
Maybe this doesn't add anything to this discussion, but I've implemented another set of extensions to eglot-x.el, and it might be telling what internal variables, functions the package uses from Eglot:
And I used advice-add on these:
Some of these are surely unnecessarily, but the lists hopefully hint the kind of API eglot-x would need from eglot. |
Beta Was this translation helpful? Give feedback.
-
Hi!
As discussed briefly in #640, I have some ideas for what eglot should look like when included in emacs 29. I'd like to start discussing this so that we can decide on what should be the best approach to take.
Right now Eglot is doing several things. The first and foremost is to be infrastructure providing encoding and decoding the lsp protocol. This is provided partly by the generic functions, but also somewhat with others helpers, such as the lsp-abiding column movement. This backstage layer is IMO what should be the primary concern when moving into core. I'd propose we'd settle on being compliant with the 3.16 spec for the first inclusion into emacs.
Next is the on-stage layer, which are the gui/dialogue/interface capabilities. This is now implemented inside the backstage layer, which I'll point out later. Emacs should proide a useble, but hot-swappable on-stage layer for users to customize. By this I mean that if I want to change some behavior related to naming or diagnostics, I should be able to do it without knowing about the spec and the internals. Much less having to make a pr to emacs. In addition, this might serve up other front-end implementations that could compete in the space of providing the best ux for emacs.
To point out how this is intertwined right now, I'll show the rename functionality currently provided by emacs:
This function handles error messaging, reading from the minibuffer, checking capabilities and making the requests. If I want to tweak this I have to provide either some entangled defcustom shenanigans to eglot itself, or I have to copy this function, figuring out what are the error bounds etc in it and implement my own. An alternative could be:
In this case the functionality is clear in the backstage/on-stage layer. The elsp-request handler is needed by everyone, but could be extended by the normal :after :before :around options. If I'd use that in the current eglot behavior, I couldn't change what's inside. Now the on-stage function
elsp-rename
can provide the ux, and just run the backstage handler. To make this point even clearer, I'd like to point out that theelsp--apply-workspace-edit
is in the on-stage layer, but could be part of the backstage layer. However, in this case I envision someone wanting to do this:As such the user has complete freedom in how to handle their own lsp implementation. Just define your own, reuse the backstage handler and off you go.
Does any of this make sense?
Beta Was this translation helpful? Give feedback.
All reactions