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 custom text document content provider #418

Merged
merged 5 commits into from
Feb 12, 2021

Conversation

evidolob
Copy link
Collaborator

@evidolob evidolob commented Jan 18, 2021

What does this PR do?

Add implementation for text document content provider which handle json-schema URI shema, to open JSON Schema in VScode editor

Note: to be able to open files from LS this PR also update vscode-languageclient to 7.0.0 version

What issues does this PR fix or reference?

#174

Is it tested? How?

@evidolob evidolob changed the title add custom text document content provider Add custom text document content provider Jan 18, 2021
@JPinkney
Copy link
Contributor

I like this approach because it means that we won't need to manually download schemas ourselves to show remote schemas. I feel like handling remote schemas, downloading them, making sure they are updated, etc would be a pain.

I guess in the end the only difference is that users on different ides/editors can't jump to remote schemas but I'm not sure how big of a deal that is

@evidolob
Copy link
Collaborator Author

@JPinkney I was thinking for adding some sort cache for remote JSON Schemas, we already have issues to support offline work - #386
Now as we load schema from vscode extension, we can cache them in to ExtensionContex#storageUri.
To keep cache valid, we can check file creation/modification time and if it was more then (for example) 1 week ago we download that file again. In ideal case we can add that time in to preference.
@JPinkney WDYT?

@JPinkney
Copy link
Contributor

That sounds like a good approach!

@evidolob evidolob marked this pull request as ready for review January 26, 2021 07:47
Signed-off-by: Yevhen Vydolob <[email protected]>
@evidolob
Copy link
Collaborator Author

@JPinkney Can you look on this PR again?

I also find that we use yaml/registerVSCodeContentRequest type, but yaml-LS expects yaml/registerContentRequest, so I change that to be alight with yaml-ls

@gorkem
Copy link
Collaborator

gorkem commented Jan 30, 2021

would using eTag make the cache more efficient?

@evidolob
Copy link
Collaborator Author

evidolob commented Feb 1, 2021

@gorkem It can, I'm was thinking on rewriting caching on this PR.

@evidolob
Copy link
Collaborator Author

evidolob commented Feb 5, 2021

@gorkem @JPinkney
Now cache uses etag header to detect that schema changed on server.

Copy link
Contributor

@JPinkney JPinkney left a comment

Choose a reason for hiding this comment

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

LGTM!

src/json-schema-content-provider.ts Outdated Show resolved Hide resolved
@evidolob evidolob merged commit 77c8e7b into redhat-developer:master Feb 12, 2021
@evidolob evidolob deleted the open-schema branch February 12, 2021 09:29
bleach31 pushed a commit to bleach31/vscode-yaml that referenced this pull request Jan 25, 2022
…priority-levels

Re-add schema priority levels
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.

3 participants