Take the drudgery out of keeping up with LSP changes with lsprotocol?

[brettcannon]

Now that LSP has a machine-readable definition, we have released lsprotocol as a generated library of the types and messages used by LSP. It uses cattrs for everything, so it's fully typed (and would address #221 ).

It's an alpha because we wanted to get this out as soon as possible to talk with you all about what would you need from the library to adopt it? We essentially want to shoulder the boring, common, sans-I/O bits that any LSP library written in Python would need so pygls and other similar libraries can focus on the more interesting stuff. To start we have just done the types and messages, but we are open to discuss taking on more common functionality that folks think makes sense.

What would you all like to see from lsprotocol to make it useful to pygls?

[tombh]

That looks awesome! To me it's obvious that Pygls should go in this direction. But I'm still new to the project so don't have the best view on it. Hopefully we can get some other perspectives.

BTW, I notice the project's links to Github aren't working, eg https://github.com/microsoft/lsprotocol

[brettcannon]

Yeah, we apparently forgot to dot the "i" in "public" on the repo. 😅 It's fixed now.

[alcarney]

It definitely makes sense to adopt something like this, the only thing I’d potentially miss from the current pydantic types is the alias feature allowing fields to be referenced using snake case (text_document) rather than camel case (textDocument). Based on a brief look at the lsprotocol repo it appears that this is not currently supported? Is it something that could be added?

Certainly not an issue if it can’t be added, just thought I’d ask :)

[karthiknadig]

The cattrs documentation has an example for this scenario, and it mentions that a better design is to use camel case attributes.
https://cattrs.readthedocs.io/en/latest/usage.html#using-factory-hooks

[mpourmpoulis]

thanks for the reply @karthiknadig

Just to clarify, my previous comment was NOT meant to question whether there are design benefits to be gained by adopting the new case convention, even though I do have some concerns regarding the applicability of recommendation in this particular context, given the complexity that might be introduced by rolling out a hacky compatibility layer (For example, regarding the mix-in idea shouldn't it also be rewriting constructors in order to accommodate for snake case keyword arguments?) But that is not up for me to decide.

My comment was instead meant to question whether there are performance incentives to drop the existing case conventions. With the translated field names already hardcoded inside the source code of the serializing logic ( just like the non-translated ones would be) it is not clear to me where meaningful performance gains should be coming from.

Lack of available time and experience with tooling meant I could only perform limited tests by adapting the benchmarking suite from cattrs, but I felt it was important to have some numbers on the table. As my machine was too noisy, I prefer to link to the results of a workflow run and in the following screenshots, taken from 3.10 runtime

• True stands for renaming/converting/translating from snake to camel case while serializing , whereas
• False stands for retaining the current case In the serialized output (so no translation).

Those numbers look tight! This seems to be in line with the reasoning above that translation does not necessarily have to be particularly expensive. pypy can of course do a bit better in the non-translated case for some tasks, but even there there appears to be variability across workflow runs.

Again due to my lack of experience, I recommend that you take those preliminary results with a grain of salt and please let me know if you find I have overlooked something either with the construction of the tests or the interpretation of the results. There is also plenty of room for improvement and if you or others interested in the discussion have already done benchmarks that are more representative of the workload you would be facing and are willing to share, please do not hesitate!

Edit: Improve wording in screenshot description

[dgreisen]

I would love the ability to support both camel and snake. Especially if @mpourmpoulis is correct that there is no real performance impact.

In my opinion, nothing quite says "this is a lazy Python wrapper around a non-native library" like a camel case api. But I also acknowledge that I regularly make design decisions based on api aesthetics that I later regret due to maintenance burden.

I'll leave the ultimate decision to the community.

Out of curiosity, what sort of edge cases could we run into with the conversion to snake case? Anything that couldn't be resolved with a manually-maintained dict of special cases?

On a separate, but related note, I would be very interested in the medium term to figure out a process by which the community makes decisions. Perhaps a separate discussion thread?

[tombh]

I started a new discussion about making community decisions: #260

[brettcannon]

Thanks, @mpourmpoulis for doing the benchmarks! I'm fine with saying that performance is not a concern and this is more about documentation and API ergonomics.

I would love the ability to support both camel and snake.

We could generate two versions of the types; camel case and snake case and let people use what they want. It's obviously a bit more work to have to generate both versions (probably in different sub-packages, e.g. lsprotocol.snake.types), but it's possible. That has the drawback, though, that any library building on top of lsprotocol couldn't be used by someone else without also using the same format (i.e. it would be the equivalent of creating two separate libraries, splitting the community between which one they prefer).

We could have attribute names that were either strict mappings (e.g. snake_name = camelName), or done via a property for one of the name formats. The problem is I don't know how messy that would be in terms of compatibility within the code (if it's just attributes and method names it's probably fine, but that's said without talking with @karthiknadig and evaluating the code). But some format would pay a penalty for any property access that we made available. I also don't know what documenting this would be like since we would have a ton of "Alias for camelName" or something to document both sets of names.

I think we are at the point we need the pygls team to tell us if they are +1/+0/-0/-1 on camel case names (as compared to snake case). For me the voting represents:

• +1 is "we wished we had gone with camel case to match the protocol to begin with"
• +0 is "camel case is nice, but we would be fine if you went with snake case"
• -0 is "we would like to stick with snake case, but if you did the PR to update the names we would go with it"
• -1 is "we will use lsprotocol, but only if it uses snake case"

I left out the "we won't use lsprotocol" as that would make this entire conversation moot. 😅

I'm also informally polling my Twitter followers to see if there's a clear trend in the community.

[karthiknadig]

We chose the camel case to align with the LSP documentation. Another reason was that translation between camel case and snake case can impact performance. If you need to add a way to covert it is possible, there are a few options. But we recommend keeping it as is for performance reasons.

If you do choose to adopt lsprotocol, we can help you with updating the usages in pygls.

[dgreisen]

We initially built this library for pythonistas to quickly learn LSP and build a language server, so we prioritized a pythonic api, part of which is snake case. However, it seems like there are now many language server developers who use PyGLS to rapidly build. For them, snake case makes the most sense.

I would like to support both. So my response would be -0.5 "if you did the PR to update the names we would merge it after re-implementing snake case ourselves."

I feel a bit more confident in that decision given the landslide in @brettcannon's twitter poll:

@tombh, @danixeee do either of you want to weigh in?

[brettcannon]

I would like to support both.

We don't have the capacity to support both, so it's one or the other.

I feel a bit more confident in that decision given the landslide in @brettcannon's twitter poll

We will be updating lsprotocol to use snake case instead of camel case based on the community's reaction here and on Twitter.

[tombh]

I'm very much a snake_case person, as was my father, and his before him. Generations of snake_case merchants! That being said, I see it as very low priority. I wouldn't want anybody going out there way to support it. If it came to it I'd vote for supporting camelCase first and exploring snake_case as and when time allowed.

[alcarney]

It feels silly for so much discussion to happen over casing and if we were starting fresh it really wouldn't matter (to me) either way. However, switching to camelCase now would be a breaking change for any existing language servers, so my vote would be for snake_case first.

There probably wouldn't be much point in forcing everyone onto camelCase only to bring back snake_case again later 🙂

[tombh]

Ah I hadn't gathered it would be a breaking change, of course that's a no go. Though I suspect it would be easy enough to autogenerate proxy code?

[karthiknadig]

New version of lsprotocol with snake case properties: https://pypi.org/project/lsprotocol/2022.0.0a2/

[gnikit]

Out of curiosity, what is the status for this PR and in general importing lsprotocol?

[tombh]

There's currently an alpha release, looking for feedback: #273

[alcarney]

#264 has become #273, and most of the big issues have been resolved, there's just the long tail of small details to tidy up.

As of a few days ago I was able to get a version of esbonio working on the v1 branch so if you're interested I'd definitely suggest checking it out.

Unfortunately though, we haven't got a migration guide yet but I'm hoping to have a draft available in the next couple of days.

Edit: Damn, @tombh beat me to it! 😅

[gnikit]

Thanks guys, I was about to start the migration of fortls to pygls when I saw lsprotocol and the v1.0.0 alpha release. Do you have a timeline for the official release? I might start a PR for fortls based on the v1 branch, but I am not that familiar with pygls.

[alcarney]

The optimist in me would like to say before the end of the year, but that will depend on how quickly the loose ends can be tidied up...