Skip to content
New issue

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

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

Already on GitHub? # to your account

Fix relative file paths with fragments #603

Merged
merged 2 commits into from
Dec 6, 2021

Conversation

codeclown
Copy link
Contributor

What does this PR do?

Fixes fragments when schema URL is a local file.

Example of such a reference:

# yaml-language-server: $schema=../../schemas/example.schema.json#/definitions/CustomSpec

Currently this doesn't work in VS Code:

image

Error on hover is:

Unable to load schema from '/path/to/schemas/example.schema.json#/definitions/CustomSpec': No content.YAML(768)

The issue is caused by "#" being turned into "%23" during some path resolve operations. This PR fixes it by excluding the fragment during those operations.

What issues does this PR fix or reference?

Couldn't find a GitHub issue.
Relates somewhat to old PR which added fragment support: #530

Is it tested? How?

Includes automated test in existing suite.
I don't know how to test this with VS Code directly (if you could advice, I'll happily test locally; as a sidenote, CONTRIBUTING.md in this regard would be helpful).

@coveralls
Copy link

coveralls commented Dec 3, 2021

Coverage Status

Coverage increased (+0.04%) to 78.649% when pulling e93cab7 on codeclown:fix-local-file-fragments into 5bafa9d on redhat-developer:main.

@evidolob
Copy link
Collaborator

evidolob commented Dec 3, 2021

@codeclown Thx for contribution.
To test this in vscode you can follow https://github.com/redhat-developer/vscode-yaml/blob/main/CONTRIBUTING.md

@codeclown
Copy link
Contributor Author

@evidolob Thanks for the tip! I managed to test the change locally, and it works as expected.

Also added an if-else to the test to account for Windows paths.

@evidolob evidolob merged commit df7931c into redhat-developer:main Dec 6, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants