Skip to content

[json] support link on $ref for external references #116540

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

Open
ionut-gheorghe opened this issue Feb 12, 2021 · 15 comments
Open

[json] support link on $ref for external references #116540

ionut-gheorghe opened this issue Feb 12, 2021 · 15 comments
Assignees
Labels
feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities json JSON support issues
Milestone

Comments

@ionut-gheorghe
Copy link

  • VS Code Version: 1.53.2
  • OS Version: Windows_NT x64 10.10.19041

image
image
image

I remember in older version that it showed the link.

Does this issue occur when all extensions are disabled?: Yes

@aeschli
Copy link
Contributor

aeschli commented Feb 12, 2021

We currently only support the link for local $ref's #....

@aeschli aeschli changed the title Json schema $ref no folow link shown for external definition [json] support link on $ref for external references Feb 12, 2021
@aeschli aeschli added json JSON support issues feature-request Request for new features or functionality labels Feb 12, 2021
@aeschli aeschli added this to the Backlog milestone Feb 12, 2021
@aeschli aeschli added the help wanted Issues identified as good community contribution opportunities label Feb 12, 2021
@yarally
Copy link

yarally commented Mar 11, 2021

Hello, is this issue currently being worked on? Otherwise I would like to check it out.

@aeschli
Copy link
Contributor

aeschli commented Mar 11, 2021

I'm not working on it, if you want to give it a go, that would be fantastic.

@yarally
Copy link

yarally commented Mar 15, 2021

Hi, after investigating for a bit I believe this feature should be added in the JSON language service. Am I correct in assuming that that we need to commit and make a PR for https://github.com/microsoft/vscode-json-languageservice? If so, should this issue be moved to there as well?

@FilipePintoReis
Copy link

@yarally how did you come to this conclusion?

@yarally
Copy link

yarally commented Mar 15, 2021

@FilipePintoReis For one of my master courses called Software Architecture we are required to learn about and contribute to a open source project. First of all I noticed that turning off the JSON language service makes it so that the support links stop functioning. Then one of my teammates found this file, which we believe handles the links of the $refs.

@aeschli
Copy link
Contributor

aeschli commented Mar 15, 2021

Yes, the change needs to be in https://github.com/microsoft/vscode-json-languageservice. We can move the issue there if that's helpful, or just create a PR there.

@yarally
Copy link

yarally commented Mar 15, 2021

Okay thanks! Keeping the issue here is fine.

@Huntervang
Copy link

Hi I'm a fellow student of @yarally and I am also currently looking into this issue. We think we will be able to implement this by adding a DocumentContext parameter to the findLinks function, similar to how the html-language-service's link finding function works. Currently this is how the html and css language services both implement DocumentContext. Would this approach be correct?

@aeschli
Copy link
Contributor

aeschli commented Mar 19, 2021

@Huntervang What you suggest makes sense!

@yarally
Copy link

yarally commented Mar 26, 2021

@aeschli We have an almost working proposal at the moment, but there is one problem that we're unsure how to solve. In order to calculate the ranges for the references, we need the respective TextDocuments. Inside the jsonServer.ts, this is done using the document manager. However, the manager only has references to documents that are currently open. Is there a way to add documents to the managers's memory or to fetch them some other way using the uri? Thanks in advance!

if (ref.uri) {
    const externalDocument = documents.get(ref.uri);
    // externalDocument is undefined if the user has not opened the file first
    if (externalDocument) {
    // Do stuff
    }

@aeschli
Copy link
Contributor

aeschli commented Mar 29, 2021

$ref points to schemas and for loading schemas we already have the schemaRequestService. I suggest to use that to load the references schema content. It will also work correctly with file URI's open in an editor.
You can then create a TextDocument and use the jsonc-parser library then will help to parse the content and find line/character of the referenced node.

The schema service uses the schemaRequestService and does caching. However, it it currently only caches the object tree, not the locations nor the underlying document.
I wouldn't add anything to the cache at the moment.

Instead, I would delay the resolving of the $ref location, so we don't need the caching. This can be done with the document link resolve feature from LSP.
For that, the link provider returns a link without target. When the link is actually clicked by the user, the resolve call is fired and allows the server to compute a target (and a correct range)
-> new API resolveLink on the json language service

@Huntervang
Copy link

If we were to only resolve the link on click this would result in us not knowing the existence of a target JSON object, and thus not knowing when the link should become a clickable object. We currently have some code which checks if the file exists and in that case creates a link. This results in the current functionality as seen below:

Here the third link is in fact not a valid link, as the specified JSON object does not exist in the targeted file.
What would be the desired behavior in such cases?

Another question we had is what would be a function that gets called when the user clicks a link within the scope of jsonServer.ts?

@aeschli
Copy link
Contributor

aeschli commented Apr 7, 2021

I would always render it as a link, if the link is syntactically correct. Even if it points to a non existing file or property.
Only when you click you will find out. It's the same with a href references in HTLM documents.
Only showing it as link if it really points to something would mean that we have to watch file changes and content changes as these could result in a link to become valid or invalid.

@aeschli
Copy link
Contributor

aeschli commented Apr 7, 2021

In jsonServer you have to add calls to connection.onDocumentLinks and connection.onDocumentLinkResolve

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality help wanted Issues identified as good community contribution opportunities json JSON support issues
Projects
None yet
Development

No branches or pull requests

5 participants