Only pass the id parameter to types that use it

[dcermak]

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

Only pass the id parameter to types that use it

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
• CONTRIBUTORS.md was updated, as appropriate
• Changelog has been updated, as needed (see CHANGELOG.md)

[tombh]

Thanks for this. Can you give an example of a request without an ID? Does it fail without the ID? Or does it get set as None?

[tombh]

Right, so the context for this issue is #312

This is ready to merge then. Would you like to add yourself to CONTRIBUTORS.md and update CHANGELOG.md?

[dcermak]

This is ready to merge then. Would you like to add yourself to CONTRIBUTORS.md and update CHANGELOG.md?

Should be done. Could you approve the CI please?

[nodeg]

@tombh Would you mind enabling the GitHub Actions for this PR, please, so we can proceed with it?

[tombh]

I don't have that option in the UI. I assume that's because it's still in Draft status?

[alcarney]

Just repeating what I said in #312, I don't think this PR is necessary.

I would argue that the behavior seen in the original issue is expected as textDocument/didOpen is a notification, not a request (see spec) and that the notify method should be used instead (or even the text_document_did_save method on the new Client when that becomes available)

That said, later on in the same thread @dcermak followed up with

Ah, thank you! I have switched to using notify, which fixes the runtime error, but sadly it creates a new one: The notify function is synchronous, but the processing of that notification happens asynchronously in the background. So the tests that run immediately after that fail, as the document has not yet been loaded. This worked with send_request, albeit of course only accidentally. Can I somehow wait for the result of that notification to be processed?

But that's probably a separate issue around writing tests involving notifications?