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.

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

Add a note about relative links in MarkupContent #1443

Open
wants to merge 3 commits into
base: gh-pages
Choose a base branch
from

Conversation

DanTup
Copy link
Contributor

@DanTup DanTup commented Apr 12, 2022

@dbaeumer is this what you had in mind for microsoft/vscode#142051 (comment)?

I don't know if MarkupContent is currently supported anywhere that isn't tied to a request for a document, but it probably makes sense to have this caveat?

@DanTup
Copy link
Contributor Author

DanTup commented Apr 12, 2022

@dbaeumer I think in the client I want to add this in protocolConverter.ts's asMarkdownString, but I don't really have access to requests there. I could pass it down through the arguments, but I guess that's either a breaking change or it'd have to be optional (which would make it easy to miss from future requests).

Is there a better way/place I can implement this? (Ideally, we could do it centrally/generically, detect the request has a document URI?).

@DanTup
Copy link
Contributor Author

DanTup commented Jun 8, 2022

@dbaeumer I started looking at this again today and realised that it's not as simple as I first thought. The documentation being rendered in a hover or code completion results might contain paths that are relative to the document containing the markdown, not relative to the document triggering the hover/completion (eg. you could be completing or hovering symbols from another file).

So I think this either needs to be handled in the servers (they rewrite the paths), or LSP allows the base URI to be included in MarkdownString? (the latter avoids the server having to parse Markdown).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants