Handle escaped colons in Windows file:// URIs #149
Conversation
|
I'm always nervous touching @natebosch @jakemac53 – thoughts? |
|
|
||
| /// Returns the index of the first character after the drive letter, or `null` | ||
| /// if [index] is not the start of a drive letter. | ||
| int? driveLetterEnd(String path, int index) { |
There was a problem hiding this comment.
[nit] What do you think about the name drivePathIndex
Can the doc call out the specific behavior when path is exactly the drive letter with no following character?
There was a problem hiding this comment.
After re-reading the implementation, I think driveLetterEnd is a fine name.
There was a problem hiding this comment.
I've update the docs, let me know if this seems ok.
There was a problem hiding this comment.
I think the change looks good. I think the risk that someone is relying on the current behavior is low, and I'm convinced by the discussion in the linked issues that this change would be a more useful implementation.
We will need a changelog entry. I would also switch to a feature version bump 1.9.0.
|
I've pushed changes + changelog/version. I left the Thanks :) |
Fixes dart-lang/path#148 Using `Uri().toFilePath()` handles escaped colons in drive letters, but `Context.fromUri()` currently does not. Allow the percent encoded colon by checking both formats and adjusting to the correct index for the following character when the percent encoding makes the drive letter longer than the typical two characters.
This is a potential fix for dart-lang/core#545. Using
Uri().toFilePath()handles escaped colons in drive letters, butContext.fromUri()currently (without this PR) does not.I'm unable to find anything concrete about how valid this is, but the opinion of the LSP maintainers is that this is valid, and since the
Uriclass handles it I feel like supporting this is the best route. I don't think there are any negative consequences of supporting this.@devoncarew @kevmoo I don't know who's getting PR notifications for this repo, but since you both committed here recently perhaps you know who might be able to review this? Thanks :-)