Repository navigation
Markup message support push - #2318
Marvin Brouwer (Marvin-Brouwer) wants to merge 8 commits into
Conversation
|
@microsoft-github-policy-service agree |
| * in pull diagnostic messages. | ||
| * | ||
| * @since 3.18.0 | ||
| * @proposed |
There was a problem hiding this comment.
Actually 3.18.0 is not proposed anymore. I think it is an oversight and can be removed.
There was a problem hiding this comment.
Clear, I'll remove it, this also answers my question in the other thread
| @@ -9052,8 +9052,12 @@ | |||
| } | |||
There was a problem hiding this comment.
Did you change the metaModel by hand or dio you generate it from the implementation?
There was a problem hiding this comment.
I generated it from the implementation, however, I did revert some additional changes that had nothing to do with mine
There was a problem hiding this comment.
I get a few of these for free when copying the metaModel from microsoft/vscode-languageserver-node:
So I just revert these specific lines, happy to put them in if you want to
|
p
…On Tue, Sep 22, Reiwa 8 at 04:14 Marvin Brouwer ***@***.***> wrote:
*Marvin-Brouwer* left a comment (microsoft/language-server-protocol#2318)
<#2318 (comment)>
@microsoft-github-policy-service agree
—
Reply to this email directly, view it on GitHub
<#2318?email_source=notifications&email_token=BZ246RSBOBY7ZWY4QRRFLOL5QF4Z5A5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTKNZWGYYDMNBYHE3KM4TFMFZW63VKON2WE43DOJUWEZLEUVSXMZLOOSWGM33PORSXEX3DNRUWG2Y#issuecomment-5766064896>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BZ246RTXJZZJKDHXPR7IJGD5QF4Z5AVCNFSNUABEKJSXA33TNF2G64TZHM2DCOJQG43DIOJ3JFZXG5LFHM2TKMZRGM2DQNJRGCQXMAQ>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The code-action guidance incorrectly treats client capabilities as server capabilities.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Adds push-diagnostic negotiation for MarkupContent messages in LSP 3.19.
Changes:
- Adds proposed push capability metadata.
- Clarifies push versus pull diagnostic guards.
- Updates generated meta-model data.
| File | Description |
|---|---|
types/diagnostic.md |
Documents transport-specific capability guards. |
metaModel/metaModel.json |
Mirrors updated protocol metadata. |
language/pullDiagnostics.md |
Clarifies pull capability scope. |
language/publishDiagnostics.md |
Adds the proposed push capability. |
language/codeAction.md |
Updates markup-message guidance. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| * and/or `textDocument.publishDiagnostics.markupMessageSupport` server capability | ||
| * before sending diagnostics with markup messages to a server. |
|
Marvin Brouwer (@Marvin-Brouwer) see the copilot review comment which is actually correct. It is a client capability telling the server whether it can send markup content or not. |
I see, so just to be sure I'm clear and we don't keep going back and forth. And if yes, it seems the sentence becomes a bit strange because of "and I suggest something like: /**
* Note that the client should check the `textDocument.diagnostic.markupMessageSupport`
* or `textDocument.publishDiagnostics.markupMessageSupport` client capabilities
* before sending diagnostics with markup messages to a server.
* Such diagnostics should be excluded for servers that don't support them.
*/Please confirm, or feel free to offer a suggestion and I'll just copy paste that. |
Specification fix for microsoft/vscode-languageserver-node#1860.
Implementation: microsoft/vscode-languageserver-node#1861
3.18 added
MarkupContentdiagnostic messages, guarded bytextDocument.diagnostic.markupMessageSupport.The guarded
messagefield is onDiagnostic, which both the push and pull models use, but the capability only exists onDiagnosticClientCapabilities.So a push-only server has no way to negotiate it.
In #1860 Dirk Bäumer (@dbaeumer)'s preference was to mirror the property rather than reword the existing one:
Targets 3.19 only.
Notes
(from Markup message support push vscode-languageserver-node#1861).
The two copies already deviate on the main branches, so a wholesale copy would have pulled in unrelated changes.
Diagnostic.messagenow has two@sincetags,so the generator emits
sinceplus asinceTagsarray, as it already does forTextDocumentEdit.edits.@proposedtag from the pull capability inpullDiagnostics.md.It was marked proposed there, but 3.18.0 has shipped.