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

Implement file scheme #404

Merged
merged 5 commits into from
Feb 20, 2024
Merged

Implement file scheme #404

merged 5 commits into from
Feb 20, 2024

Conversation

pjbull
Copy link
Member

@pjbull pjbull commented Feb 18, 2024

Simple implementation of file: scheme in AnyPath.

Uses implementation from here, but it looks like there is a from_uri method coming to pathlib in 3.13.

Closes #401

@pjbull pjbull requested a review from jayqi February 18, 2024 18:52
Copy link
Contributor

github-actions bot commented Feb 18, 2024

@github-actions github-actions bot temporarily deployed to pull request February 18, 2024 18:53 Inactive
@github-actions github-actions bot temporarily deployed to pull request February 18, 2024 18:57 Inactive
Copy link

codecov bot commented Feb 18, 2024

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (ddc3b94) 94.0% compared to head (3ebacd1) 94.0%.

Additional details and impacted files
@@          Coverage Diff           @@
##           master    #404   +/-   ##
======================================
  Coverage    94.0%   94.0%           
======================================
  Files          22      23    +1     
  Lines        1619    1637   +18     
======================================
+ Hits         1522    1540   +18     
  Misses         97      97           
Files Coverage Δ
cloudpathlib/anypath.py 91.3% <100.0%> (+1.3%) ⬆️
cloudpathlib/url_utils.py 83.3% <83.3%> (ø)

... and 1 file with indirect coverage changes

@github-actions github-actions bot temporarily deployed to pull request February 18, 2024 19:32 Inactive
Copy link
Member

@jayqi jayqi left a comment

Choose a reason for hiding this comment

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

Do we correctly handle percent-encoded colons from a drive on Windows? e.g.,

file://c:/path/to/file.txt vs. file://c%3A/path/to/file.txt

We should at least have a test for it.

See this PR AcademySoftwareFoundation/OpenTimelineIO#1664

that also referenced the cpython issue that you took this implementation from. They discuss the issue in that PR and have a Windows test case.

I tried to search around, and I found this which suggests that the percent-encoded version should be considered valid: https://github.com/microsoft/language-server-protocol/pull/1786/files

@github-actions github-actions bot temporarily deployed to pull request February 20, 2024 14:26 Inactive
@pjbull
Copy link
Member Author

pjbull commented Feb 20, 2024

Updated, thanks @jayqi!

@pjbull pjbull merged commit 674f1db into master Feb 20, 2024
28 checks passed
@pjbull pjbull deleted the 401-file-scheme branch February 20, 2024 21:11
@jayqi jayqi mentioned this pull request Mar 7, 2024
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.

Support file: ?
2 participants