Use MarkupContent in diagnostic message if supported by client

[jwortmann]

I noticed that most diagnostic messages from JETLS use inline code in standard Markdown format (delimited by backticks).

The upcoming 3.18 version of the LSP specs allows to send diagnostic messages with MarkupContent if the client has support for that:

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

This PR proposes to use MarkupContent in diagnostic messages if applicable for the message and if supported by the client. This allows the client to render the message in a nicer way.

The following screenshots are from Sublime Text and with a currently unreleased version of the LSP client, which has recently added support for that (it will be available with the next release of the LSP package for Sublime Text).

Before:

---

After:

---

I want to mention that technically the client capability for markupMessageSupport is only part of the DiagnosticClientCapabilities (textDocument.diagnostic.markupMessageSupport), and not part of the PublishDiagnosticsClientCapabilities (textDocument.publishDiagnostics). However, I believe this is just a small inconsistency in the LSP specs, and it wouldn't really make sense for the client to limit this to only pull diagnostics, so I also applied the MarkupContent to diagnostics from publishDiagnostics notifications. The docstring for the Diagnostic type that I quoted above also refers only to that single capability path, so I assume that it should be fine.

[codecov]

Codecov Report

❌ Patch coverage is 33.33333% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.29%. Comparing base (5d7b2bf) to head (f199302).

Files with missing lines	Patch %	Lines
src/diagnostic.jl	30.76%	9 Missing ⚠️
src/app/cli-check.jl	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #633      +/-   ##
==========================================
- Coverage   69.04%   67.29%   -1.75%     
==========================================
  Files          51       63      +12     
  Lines        8813     9195     +382     
==========================================
+ Hits         6085     6188     +103     
- Misses       2728     3007     +279     

Flag	Coverage Δ
JETLS.jl	68.98% <28.57%> (-0.07%)	⬇️
LSP.jl	27.22% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[aviatesk]

Nice improvement. I hadn't looked into LSP 3.18 details yet, but I think clients will gradually add support for it, so I really appreciate you making this change.

I left a few comments.

To merge this, please also fix the selfcheck errors (you'll need to modify cli-check.jl): https://github.com/aviatesk/JETLS.jl/actions/runs/24489449489/job/71571460604?pr=633
To run the selfcheck locally, you can just run ./scripts/selfcheck from the root of this repo.
Don't worry about the vendor-deps CI failure.

[aviatesk]

Suggested change

	if diagnostic.message isa String && occursin('`', diagnostic.message)
	if diagnostic.message isa String

Markdown syntax isn't limited to backticks, so we could skip this condition and just replace all the diagnostics with the ones that use the MarkupContent version. Since Diagnostic is an immutable struct, the performance cost should be negligible.

[jwortmann]

Yes, but this also carries a cost at the client side, which needs to pass diagnostic messages into a Markdown parser in that case. For the example from the screenshot showing a popup on mouse hover this should be negligible, but in Sublime Text we also have an alternative mode to permanently display diagnostic messages on the right side of the line directly in the editor:

In that case it could be useful to skip the Markdown parsing if not necessary.

Do you know that you actually use other Markdown features than the inline code in some of the diagnostic messages?

[aviatesk]

There aren't many, but some of the top-level error messages returned by full-analysis do contain things like bullet points. Those would be better rendered as markdown.
More of these might be added in the future, and it's a bit inconvenient to have to keep this check in mind every time we add new diagnostics.

The performance concern for inline diagnostics is valid, but since the capability is being declared, it feels unnatural to keep code on the server side that's driven by client-side performance concerns.
I think a better solution would be for Sublime to just skip markdown rendering entirely for inline diagnostics?

[jwortmann]

Okay, should be fine I guess. In case it turns out to have performance implications for inline diagnostics, I can take action on the client side.

[aviatesk]

Thanks so much for this enhancement.