UTF-8 and UTF-32 position encoding support

[tombh]

This allows editors to use position encodings other than the default of UTF-16. This contributes to LSP 3.17 support. See https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#positionEncodingKind

Code review checklist (for code reviewer to complete)

• Pull request represents a single change (i.e. not fixing disparate/unrelated things in a single PR)
• Title summarizes what is changing
• Commit messages are meaningful (see this for details)
• Tests have been included and/or updated, as appropriate
• Docstrings have been included and/or updated, as appropriate
• Standalone docs have been updated accordingly

[alcarney]

I can't comment on the actual implementation of UTf-8 and UTF-32 support, but overall the approach looks fine to me.

Just added some notes around maintaining backwards compatibility

[karthiknadig]

@tombh I published a build of lsprotocol with the fix for PositionEncodingKind

[tombh]

Thanks Karthik, I did see that, thank you so much. I've just been busy and haven't had a chance to update this PR yet.

Once this is merged we can make a new release with LSP 3.17 support!

[tombh]

@alcarney I've pushed those changes. Also, because I'm trying to follow strict typing from now on, I made a change to the Server, LanguageServer classes, just moving the lsp field. I think it was to do with the converter work you did. Basically it was to better reflect that JsonRPCProtocol takes LanguageServer not Server. I isolated the change in the chore: move Server.lsp to LanguageServer.lsp commit, does that look ok?

[alcarney]

I've pushed those changes.

Thanks, they look great!

it was to better reflect that JsonRPCProtocol takes LanguageServer not Server

Does it have to though? Ideally, there shouldn't be anything in JsonRPCProtocol that depends on LanguageServer directly, since we have the LanguageServerProtocol class to hold the details specific to LSP.

I'm sure that is not the reality though! Otherwise you wouldn't have had to update the type annotations! 😄
But IMO if we have to change anything we should be trying to move to a place where JsonRPCProtocol can be paried with Server, just as LanguageServerProtocol is paired with LanguageServer.

Otherwise that base Server class is quite useless on it's own 😅

---

Sort of related I've been meaning to post some thoughts in #334 on how we apply what we've learned with the JsonRPCClient to the server side of pygls (e.g. autogenerated BaseLanguageServer class), move to the high level asyncio APIs and generally clean up all these little inconsistencies we're finding.

It'll be mid-longer term goal though (probably resulting in a v2?!) . Maybe I'll try and get something written down in the next few days so we have something concrete to discuss...

[tombh]

Does it have to though? Ideally, there shouldn't be anything in JsonRPCProtocol that depends on LanguageServer directly, since we have the LanguageServerProtocol class to hold the details specific to LSP.

That totally makes sense. I've removed the commit for now and just ignored the type. I'll rejig it in the direction you mention another time. For the record these are the methods that need moving:

pygls/protocol.py:390: error: "Server" has no attribute "_report_server_error"  [attr-defined]
pygls/protocol.py:412: error: "Server" has no attribute "_report_server_error"  [attr-defined]
pygls/protocol.py:423: error: "Server" has no attribute "_report_server_error"  [attr-defined]
pygls/protocol.py:538: error: "Server" has no attribute "_report_server_error"  [attr-defined]
pygls/protocol.py:579: error: "Server" has no attribute "_report_server_error"  [attr-defined]
pygls/protocol.py:810: error: "Server" has no attribute "process_id"  [attr-defined]
pygls/protocol.py:812: error: "Server" has no attribute "_text_document_sync_kind"; maybe "text_document_sync_kind"?  [attr-defined]
pygls/protocol.py:813: error: "Server" has no attribute "_notebook_document_sync"  [attr-defined]

Sort of related I've been meaning to post some thoughts in #334 on how we apply what we've learned with the JsonRPCClient to the server side of pygls (e.g. autogenerated BaseLanguageServer class), move to the high level asyncio APIs and generally clean up all these little inconsistencies we're finding.

How interesting! That in emulating a client you've learnt how to improve the server. Really cool.

[alcarney]

Looks good to me! 😄

[alcarney]

For the record these are the methods that need moving:

Awesome thanks! They don't look too bad to fix, _report_server_error is generic enough that it could probably be moved into Server? - just with a non-LSP default reporting mechanism.
And the others look like a type annotation onserver here would be enough to fix them?

[tombh]

Yeah, should be pretty easy (touch wood)