plots: quote and resolve paths properly#5670
Merged
Merged
Conversation
2 tasks
3de011b to
2b689d4
Compare
pared
approved these changes
Mar 23, 2021
Collaborator
Author
|
Need to do a QA on Windows for the file-uri changes. Previously, the file-uri where being constructed through platform-specific paths, which means there were |
This commit fixes 3 things: 1. The path printed on the terminal is properly is quoted. This was problematic when the output is piped to `xdg-open` if the path contained spaces or quotes or special characters. 2. The path is resolved. This is not an issue at all, but makes the output nicer. 3. The path with which the browser is opened with uses relative path. This is done to solve issue in WSL, that the absolute path is not accessible outside the WSL for host programs as the linux filesystem is available on `/wsl$` path.
2b689d4 to
f390aba
Compare
Collaborator
Author
|
Was not able to test out on windows, I was curious if it worked before there, but probably it didn't. 🤞🏼 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit fixes 3 things:
problematic when the output is piped to
xdg-openif the path containedspaces or quotes or special characters.
output look nicer.
This is done to solve issue in WSL, that the absolute path is not
accessible outside the WSL for host programs as the linux filesystem
is available on
/wsl$path.os.path.join. This has been changed to useas_uri()frompathlib.Path.With the fix to use relative path as argument to
webbrowser.open, I expect WSL to be able to work from outside (tested) as well as within inside if needed (didnot test at all). This does work on Linux.See #5584 (comment)
❗ I have followed the Contributing to DVC checklist.
📖 If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here.
Thank you for the contribution - we'll try to review it as soon as possible. 🙏