Add name and version arguments to LanguageServer()

[nemethf]

These are sent as serverInfo in InitializeResult

Close #274

Description (e.g. "Related to ...", etc.)

Although we discussed the possibility of mandating these arguments in #274, this PR does not make the new name and version arguments of the constructor of class LanguageServer mandatory. So, it can be merged to the current master before the release of v1.0.0.

When the name is not set, the server still sends the serverInfo as null. I'm not sure that's the right thing to do. I can update the code to omit the serverInfo field altogether in this case. What do you think?

Code review checklist (for code reviewer to complete)

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

[tombh]

Thank you very much for this. I would like to merge it as is (just needs the flake8 lint changes I think), but what do you think about also adding a warning log? Something like that a future version will make these values non-optional? I'm still relatively new to the project, so I don't know what the best way to do it is. For example I don't think that the LSP client needs to know that the sever was started without a name and version, but it would be good if the server's internal logs mentioned it. So I think the logger in server.py:40 might be appropriate?

Edit: BTW, was I right when I wrote this in the issue?

... they make me think of 2 very valid points. Both of which relate to encouraging best practices. 1. A language server should always broadcast a unique name to clients. A user inside their editor should be able to easily identify a server in order to say, restart it or filter its logs. 2. Language server developers would benefit from being able to easily and consistently know the version of the server that a user is reporting an issue for.

If so, might it be good to explain why these values are slated for becoming non-optional?

[nemethf]

(just needs the flake8 lint changes I think), but what do you think about also adding a warning log?

I fixed the lint errors and added a warning as you suggested.

I don't think that the LSP client needs to know that the sever was started without a name and version

Hm. If we really want to over-engineer this, then the next version(s) should log a warning as the PR does, and the version right before the version that actually mandates this info should send a window/showMessage notification about the forthcoming change.

Edit: BTW, was I right when I wrote this in the issue?

... they make me think of 2 very valid points. Both of which relate to encouraging best practices. 1. A language server should always broadcast a unique name to clients. A user inside their editor should be able to easily identify a server in order to say, restart it or filter its logs. 2. Language server developers would benefit from being able to easily and consistently know the version of the server that a user is reporting an issue for.

The second point is definitely valid and basically that was one of my motivation for this PR. For the first point I'd say something like "A user inside their editor should be able to easily identify a server in order to know what server the editor has stated automatically (if multiple servers are eligible for a given source file.)"

It is true that usually the editor can find out the name and the version of the server by some other means, but the serverInfo is the standardized way for the server to convey this information to the client.

The warning message points to this discussion, so the developers could express their thoughts about the change here.

[tombh]

This is great, thank you. And thanks for the responses and extra context.

and the version right before the version that actually mandates this info should send a window/showMessage notification about the forthcoming change

Agreed.