Add an optional version parameter to LanguageServerProtocol publish_diagnostics

[pyscripter]

• The LSP v15 added an optional version parameter to the publishDiagnostics notification.
• PublishDiagnosticsParams includes version but,
• LanguageServerProtocol.publish_diagnostics is not using version

Please make version an optional parameter of LanguageServerProtocol.publish_diagnostics so that Lsp Servers can implement this feature.

[tombh]

I assume this is fixed in the recent v1 release, as it now depends on the official lsprotocol module from Microsoft. Let us know if there are still any issues.

[pyscripter]

I think the problem still exists.

If you look at server.py, publish_diagnostics does not include the version as an optional parameter.

Same is true in protocol.py

    def publish_diagnostics(self, doc_uri: str, diagnostics: List[Diagnostic]) -> None:
        """Sends diagnostic notification to the client."""
        self.notify(TEXT_DOCUMENT_PUBLISH_DIAGNOSTICS,
                    PublishDiagnosticsParams(uri=doc_uri, diagnostics=diagnostics))

You need to pass the version as a parameter in publish_diagnostics and intitialize PublishDiagnosticsParams using version.

Please reopen the issue.

[tombh]

Oh I accidentality closed this by mistake! Thanks for reminding me.

[pyscripter]

Two changes are needed:

server.py

    def publish_diagnostics(self, doc_uri: str, diagnostics: List[Diagnostic], version: Optional[int] = None ) :
        """Sends diagnostic notification to the client."""
        self.lsp.publish_diagnostics(doc_uri, diagnostics, version)

protocol.py

    def publish_diagnostics(self, doc_uri: str, diagnostics: List[Diagnostic], version: Optional[int] = None ) -> None:
        """Sends diagnostic notification to the client."""
        Params = PublishDiagnosticsParams(uri=doc_uri, diagnostics=diagnostics, version=version)
        self.notify(TEXT_DOCUMENT_PUBLISH_DIAGNOSTICS, Params)

I have tested and it works well. If you approve. I can submit a PR.

[tombh]

Now that I look at it, I think it's clear that the API needs to be this:

server.py

    def publish_diagnostics(self, params: PublishDiagnosticsParams) :
        """Sends diagnostic notification to the client."""
        self.lsp.publish_diagnostics(params)

protocol.py

    def publish_diagnostics(self, params: PublishDiagnosticsParams) -> None:
        """Sends diagnostic notification to the client."""
        self.notify(TEXT_DOCUMENT_PUBLISH_DIAGNOSTICS, params)

But I'm not sure why there have to be 2 such similar methods. Can't it even just be this?
server.py

    def publish_diagnostics(self, params: PublishDiagnosticsParams) :
        """Sends diagnostic notification to the client."""
        self.lsp.notify(TEXT_DOCUMENT_PUBLISH_DIAGNOSTICS, params)

But that's all breaking changes anyway. It's a shame we didn't include it with the other breaking changes in the recent v1 release. I'm not sure of the best way to introduce deprecation. How about something like this?

server.py

def publish_diagnostics(
    self,
    params_or_uri: Union[str, PublishDiagnosticsParams],
    diagnostics: Optional[List[Diagnostic]],
    version: Optional[int],
):
    """Sends diagnostic notification to the client."""
    params = self.lsp._publish_diagnostics_deprecator(params_or_uri, diagnostics, version)
    self.lsp.notify(TEXT_DOCUMENT_PUBLISH_DIAGNOSTICS, params)

protocol.py

def _publish_diagnostics_deprecator(
    self,
    params_or_uri: Union[str, PublishDiagnosticsParams],
    diagnostics: Optional[List[Diagnostic]],
    version: Optional[int],
) -> PublishDiagnosticsParams:
    if isinstance(params_or_uri, str):
        message = "DEPRECATION: "
        "`publish_diagnostics(self, doc_uri: str, diagnostics: List[Diagnostic], version: Optional[int] = None)`"
        "will be replaced with `publish_diagnostics(self, params: PublishDiagnosticsParams)`"
        self.logger.warning(message)

        params = PublishDiagnosticsParams(
            uri=params_or_uri, diagnostics=diagnostics, version=version
        )
    else:
        params = params_or_uri
    return params

def publish_diagnostics(
    self,
    params_or_uri: Union[str, PublishDiagnosticsParams],
    diagnostics: Optional[List[Diagnostic]],
    version: Optional[int],
):
    """Sends diagnostic notification to the client."""
    params = self._publish_diagnostics_deprecator(params_or_uri, diagnostics, version)
    self.notify(TEXT_DOCUMENT_PUBLISH_DIAGNOSTICS, params)

[pyscripter]

This also looks good.

But bear in mind that the changes I submitted are simpler and backward compatible (they do not break existing code). Existing servers can choose to provide version info to publish_diagnostics, if and when they want. version is an optional parameter.

[tombh]

I've made a PR at #303, I think it lets us have our cake and eat it?

[pyscripter]

Looks good.