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

Rework Uri.parse and Uri.toString #83060

Merged
merged 23 commits into from
Oct 24, 2019
Merged

Rework Uri.parse and Uri.toString #83060

merged 23 commits into from
Oct 24, 2019

Conversation

jrieken
Copy link
Member

@jrieken jrieken commented Oct 22, 2019

tl;dr - This PR is about reimplementing our Uri.parse and Uri.toString to be more standard compliant (or closer to what users expect) but without breaking existing code that relies on its quirks.

The overall URI situation is unhappy because when VS Code started the "offical" URL global didn't exit yet. Therefore we have our own URI implementation - it serves us as data container for the typical uri components: scheme, authority, path, query, and fragment (see https://tools.ietf.org/html/rfc3986#section-3). To make dealing with UNC path simpler we added fsPath to our implementation. Also, our implementation of URI.toString normalises the outcome, e.g. percent encoded sequences are upper-cased, windows drive letters are lower-cased etc, and that allows its result to be used as key in hash maps.

The needs of VS Code are fulfilled with the current implementation and generally trouble free. Issues happen when "our" URIs leave VS Code, e.g to a browser to open a link. Then issues with encoding/decoding happen because we encode too much. On top, our implementation decodes on parse (whereas the WHATWG url encodes on parse) and can potentially loose data along the route by not re-encoding things.

Given tight constraints and the high risk of breakage we cannot fix all issues but the following are aimed at

@jrieken jrieken self-assigned this Oct 22, 2019
@jrieken jrieken added this to the October 2019 milestone Oct 22, 2019
@jrieken jrieken added the uri label Oct 22, 2019
src/vs/base/common/uri.ts Outdated Show resolved Hide resolved
@jrieken jrieken merged commit 30886e2 into master Oct 24, 2019
@jrieken jrieken deleted the joh/uri-parse branch October 24, 2019 10:22
jrieken added a commit that referenced this pull request Oct 30, 2019
This reverts commit 30886e2, reversing
changes made to 525f1e4.
@github-actions github-actions bot locked and limited conversation to collaborators Mar 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants