-
Notifications
You must be signed in to change notification settings - Fork 38.2k
Description
This is the continuation of: 7d4cd4a#r37940098.
Background: We can classify uris into two categories
- Those that have posix-paths, like
file:///foo/bar.bazzorvscode-remote://bing+bong/boo/far/fazz, and - those with paths without special/known semantics, like
untitled:unitiled-1orcommand:deleteLeft
Uris of both categories exist in our system, generally uris that represent files are posix style, e.g all file-uris and all uris from contributed file systems. However, uris for virtual documents, command uris, and others don't fall into this category.
The URI class knows a little about this and for instance enforces that posix style uri paths start with a slash, e.g URI.parse('file:foo').toString() === 'file:///foo' whereas that "fixing" doesn't happen for arbitrary schemes (only file, http, https and arguably we should add vscode-remote here)
Now, the following problems exist:
- We have the
resources.ts-utility which offers many convenience functions when working with uris, e.g ways to compare them. But these utilities are all written with the assumption that it works on uris with posix-paths, e.g it states thatcommand:/deleteLeftequalscommand:deleteLeft(which breaks compatibility withuri.toString-usages). To make the confusion perfect it doesn't check, enforce, nor document that it work only works on a subset of uris, e.g. blind adoptions like these dc0ab50 made a perfect mess - We have alternative, also fishy, ways to compare uris of which I know
ResourceMap<T>and the widespread use ofmap.set(uri.toString(), 42)
I think we need to first decide if
- the
resources.tsutility should only work on uris with posix-paths. That means we need at least documentation, better enforcement, and we need to review or revert @mjbvz's "adoption". - alternatively we make the
resources.tsutility be fit for all uris, esp with respect to uri comparisons. That means we need to know when posix rules apply and when not.
I am strongly suggesting to use variant 2 because anything else will be confusing.
As a next step we need to use the same URI-equivalence checks, e.g it's bad that the renderer uses a different identity for documents then the extension host. The ResourcesMap<T> also uses a different identity than for instance users of getComparisonKey. We should align that and ideally have them all root getComparsionKey and/or isEqual. So, this is what I think needs to be done
- use
getComparisonKeyinResourceMap<T>@bpasero - use
ResourceMapwhenever storing something by URI - use the same logic in
TernarySearchTree@forPathswhich actually isforUris@jrieken - define what makes uris equal, e.g. scheme, authority are case insensitive, path is case sensitive or not (depends on the OS, scheme), query is case sensitive, fragment is ignored. Also don't massage paths during compare but during creation of uris. @aeschli