Cannot make TEXT_DOCUMENT_DID_SAVE work after upgrading to 1.0.2

[mturoci]

I am trying to register a handler for saving the file

@server.feature(TEXT_DOCUMENT_DID_SAVE, TextDocumentSaveRegistrationOptions(include_text=True))
def did_save(ls: WaveLanguageServer, params: DidSaveTextDocumentParams):

This worked fine in 0.11.0, but throws the following error in 1.0.2.

./venv/bin/python -m server --tcp
Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "/Users/martinturoci/go/src/github.com/h2oai/telesync/tools/vscode-extension/server/__main__.py", line 25, in <module>
    from .lsp_server import server
  File "/Users/martinturoci/go/src/github.com/h2oai/telesync/tools/vscode-extension/server/lsp_server.py", line 86, in <module>
    @server.feature(TEXT_DOCUMENT_DID_SAVE, TextDocumentSaveRegistrationOptions(include_text=True))
     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/martinturoci/go/src/github.com/h2oai/telesync/tools/vscode-extension/venv/lib/python3.11/site-packages/pygls/feature_manager.py", line 172, in decorator
    options_type = get_method_options_type(feature_name)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/martinturoci/go/src/github.com/h2oai/telesync/tools/vscode-extension/venv/lib/python3.11/site-packages/pygls/lsp/__init__.py", line 104, in get_method_options_type
    raise MethodTypeNotRegisteredError(method_name)
pygls.exceptions.MethodTypeNotRegisteredError: textDocument/didSave
make: *** [run-server] Error 1

Any ideas?

Note TEXT_DOCUMENT_DID_SAVE is imported from lsprotocol.types.

[alcarney]

It looks like you’ve found a bug :/

pygls tries to figure out the correct options type that should go in the @server.feature decorator. Unfortunately it’s not able to do that in your case since it appears that TEXT_DOCUMENT_DID_SAVE doesn’t follow the same pattern as the other methods.

That said, it should be an easy fix :)

Side note: TextDocumentSaveRegistrationOptions isn’t the right type to be using here (and it looks like pygls v0.11 would incorrectly force you to use this type), it should be SaveOptions instead.

To the best of my knowledge, pygls only uses the option types to populate the ServerCapabilities field of its response to the initialize request, which requires it to use SaveOptions.

TextDocumentSaveRegistrationOptions however, is meant to be used when using the dynamic register/unregister capabilities part of the LSP spec, so contains additional fields for that use case.

Anyway I am being pedantic here (it’s worked for you up until now hasn’t it 😄) and just giving a heads up that when the fix does land, pygls will likely want you to switch to using SaveOptions

[mturoci]

Thanks @alcarney!

just giving a heads up that when the fix does land, pygls will likely want you to switch to using SaveOptions

Fair enough. I am good with anything that makes it work :) So IIUC, there is no workaround to make TEXT_DOCUMENT_DID_SAVE work as of now, correct?

Btw shall I file an issue or can I leave that up to you?

[alcarney]

there is no workaround to make TEXT_DOCUMENT_DID_SAVE work as of now, correct?

Actually, the following looks like it should work

from pygls.lsp import METHOD_TO_OPTIONS

METHOD_TO_OPTIONS[TEXT_DOCUMENT_DID_SAVE] = TextDocumentSaveRegistrationOptions

@server.feature(TEXT_DOCUMENT_DID_SAVE, TextDocumentSaveRegistrationOptions(include_text=True))
def did_save(ls: WaveLanguageServer, params: DidSaveTextDocumentParams):
    ...

Replace TextDocumentSaveRegistrationOptions with SaveOptions and you're basically implementing the bug fix I'm going to do yourself :)

Btw shall I file an issue or can I leave that up to you?

I wouldn't worry about it, we can probably get away with just referencing this discussion :)

[mturoci]

Perfect, will give it a try tomorrow. Thanks a ton @alcarney, a truly first-class OSS experience :)

[mturoci]

Workaround works like a charm @alcarney. Looking forward to the next release.

[tombh]

BTW you can also use the current master branch (with the fix) of Pygls in your project with things like:

pip install git+https://github.com/openlawlibrary/pygls@95f0d5d
pygls==git+https://github.com/openlawlibrary/pygls@95f0d5d

...where 95f0d5d is the particular commit hash that you're interested in.

[frabert]

Hello! I found myself needing to do a similar thing for a different method:

METHOD_TO_OPTIONS[WORKSPACE_DID_CHANGE_WATCHED_FILES] = DidChangeWatchedFilesRegistrationOptions