Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Specification for MarkupContent support in diagnostic messages #1905

Merged
merged 2 commits into from
Mar 19, 2024
Merged

Specification for MarkupContent support in diagnostic messages #1905

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
31abb4a
add markup diagnostic message spec
MariaSolOs Mar 1, 2024
09ea98e
Update metamodel
MariaSolOs Mar 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions _specifications/lsp/3.18/general/initialize.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -885,6 +885,28 @@ interface ServerCapabilities {
*/
inlineCompletionProvider?: boolean | InlineCompletionOptions;

/**
* Text document specific server capabilities.
*
* @since 3.18.0
*/
textDocument?: {
/**
* Capabilities specific to the diagnostic pull model.
*
* @since 3.18.0
*/
diagnostic?: {
/**
* Whether the server supports `MarkupContent` in diagnostic messages.
*
* @since 3.18.0
* @proposed
*/
markupMessageSupport?: boolean;
};
};

/**
* Workspace specific server capabilities
*/
Expand Down
3 changes: 3 additions & 0 deletions _specifications/lsp/3.18/language/codeAction.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,9 @@ export interface CodeActionContext {
* for the given range. There is no guarantee that these accurately reflect
* the error state of the resource. The primary parameter
* to compute code actions is the provided range.
*
* Note that the client should check the `textDocument.diagnostic.markupMessageSupport`
* server capability before sending diagnostics with markup messages to a server.
*/
diagnostics: Diagnostic[];

Expand Down
8 changes: 8 additions & 0 deletions _specifications/lsp/3.18/language/pullDiagnostics.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,14 @@ export interface DiagnosticClientCapabilities {
* pulls.
*/
relatedDocumentSupport?: boolean;

/**
* Whether the client supports `MarkupContent` in diagnostic messages.
*
* @since 3.18.0
* @proposed
*/
markupMessageSupport?: boolean;
}
```

Expand Down
63 changes: 59 additions & 4 deletions _specifications/lsp/3.18/metaModel/metaModel.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8736,6 +8736,41 @@
"since": "3.18.0",
"proposed": true
},
{
"name": "textDocument",
"type": {
"kind": "literal",
"value": {
"properties": [
{
"name": "diagnostic",
"type": {
"kind": "literal",
"properties": [
{
"name": "markupMessageSupport",
"type": {
"kind": "base",
"name": "boolean"
},
"optional": true,
"documentation": "Whether the server supports `MarkupContent` in diagnostic messages.",
"since": "3.18.0",
"proposed": true
}
]
},
"optional": true,
"documentation": "Capabilities specific to the diagnostic pull model.\n\n@since 3.18.0",
"since": "3.18.0"
}
]
}
},
"optional": true,
"documentation": "Text document specific server capabilities.\n\n@since 3.18.0\n@proposed",
"since": "3.18.0"
},
{
"name": "workspace",
"type": {
Expand Down Expand Up @@ -8920,10 +8955,19 @@
{
"name": "message",
"type": {
"kind": "base",
"name": "string"
"kind": "or",
"items": [
{
"kind": "base",
"name": "string"
},
{
"kind": "reference",
"name": "MarkupContent"
}
]
},
"documentation": "The diagnostic's message. It usually appears in the user interface"
"documentation": "The diagnostic's message. It usually appears in the user interface.\n\n@since 3.18.0 - support for `MarkupContent`. This is guarded by the client capability `textDocument.diagnostic.markupMessageSupport`."
},
{
"name": "tags",
Expand Down Expand Up @@ -9388,7 +9432,7 @@
"name": "Diagnostic"
}
},
"documentation": "An array of diagnostics known on the client side overlapping the range provided to the\n`textDocument/codeAction` request. They are provided so that the server knows which\nerrors are currently presented to the user for the given range. There is no guarantee\nthat these accurately reflect the error state of the resource. The primary parameter\nto compute code actions is the provided range."
"documentation": "An array of diagnostics known on the client side overlapping the range provided to the\n`textDocument/codeAction` request. They are provided so that the server knows which\nerrors are currently presented to the user for the given range. There is no guarantee\nthat these accurately reflect the error state of the resource. The primary parameter\nto compute code actions is the provided range.\n\nNote that the client should check the `textDocument.diagnostic.markupMessageSupport` server capability before sending diagnostics with markup messages to a server."
},
{
"name": "only",
Expand Down Expand Up @@ -12870,6 +12914,17 @@
},
"optional": true,
"documentation": "Whether the clients supports related documents for document diagnostic pulls."
},
{
"name": "markupMessageSupport",
"type": {
"kind": "base",
"name": "boolean"
},
"optional": true,
"documentation": "Whether the client supports `MarkupContent` in diagnostic messages.",
"since": "3.18.0",
"proposed": true
}
],
"documentation": "Client capabilities specific to diagnostic pull requests.\n\n@since 3.17.0",
Expand Down
9 changes: 8 additions & 1 deletion _specifications/lsp/3.18/types/diagnostic.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
#### <a href="#diagnostic" name="diagnostic" class="anchor"> Diagnostic </a>

- New in version 3.18: support for markup content in diagnostic messages. The support is guarded by the
client capability `textDocument.diagnostic.markupMessageSupport`. If a client doesn't signal the capability,
servers shouldn't send `MarkupContent` diagnostic messages back to the client.

Represents a diagnostic, such as a compiler error or warning. Diagnostic objects are only valid in the scope of a resource.

```typescript
Expand Down Expand Up @@ -35,8 +39,11 @@ export interface Diagnostic {

/**
* The diagnostic's message.
*
* @since 3.18.0 - support for MarkupContent. This is guarded by the client
* capability `textDocument.diagnostic.markupMessageSupport`.
*/
message: string;
message: string | MarkupContent;
Copy link
Contributor

@rchl rchl Mar 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be a breaking change (type-wise) if the type suddenly changes from string to string | MarkupContent. I think this might need to be a separate property.

(not sure how such cases were handled historically)

Copy link
Contributor

@rchl rchl Mar 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Realistically servers will probably have to support both plain string and markup since there is no guarantee that all clients will support markup. So it might make sense for servers to provide both variants in separate properties (when client supports markup).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be a breaking change (type-wise) if the type suddenly changes from string to string | MarkupContent. I think this might need to be a separate property.

I think this is fine given that it is behind a capability. Note how previous proposals followed the same approach; e.g. snipped edits.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Realistically servers will probably have to support both plain string and markup since there is no guarantee that all clients will support markup. So it might make sense for servers to provide both variants in separate properties (when client supports markup).

True, although one could argue that this is also true for other features that support markup content, yet those don't provide both formats.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That being said, @dbaeumer has the final call.


/**
* Additional metadata about the diagnostic.
Expand Down