Skip to content

Implement file scheme - #404

Merged
pjbull merged 5 commits into
masterfrom
401-file-scheme
Feb 20, 2024
Merged

pjbull merged 5 commits into
masterfrom
401-file-scheme

Conversation

@pjbull

@pjbull pjbull commented Feb 18, 2024

Copy link
Copy Markdown
Member

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
@github-actions

github-actions Bot commented Feb 18, 2024 •

Copy link
Copy Markdown
Contributor

@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
@codecov

codecov Bot commented Feb 18, 2024 •

Copy link
Copy Markdown

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

@jayqi jayqi left a comment

Copy link
Copy Markdown
Member

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

pjbull commented Feb 20, 2024

Copy link
Copy Markdown
Member Author

Updated, thanks @jayqi!

@pjbull
pjbull merged commit 674f1db into master Feb 20, 2024
@pjbull
pjbull deleted the 401-file-scheme branch February 20, 2024 21:11
@jayqi jayqi mentioned this pull request Mar 7, 2024

This branch was previously deployed

1 inactive deployment
pull request — 3ebacd14 Deployed Feb 20, 2024 by github-actions[bot]
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