Storage: workspace identifier changed (URI.toString())

[bpasero]

We are storing all workspace storage for folders with a prefix that is computed from the folder URI.toString(). This key has changed on Windows from encoding the colon at the drive letter to no longer encode. Given that, no workspace storage is found anymore.

@jrieken please advise what I can do here to get back the old behaviour. Maybe we need some utility function. I would also suggest to do a check of clients of URI.toString() to see if anyone is actually persisting this data and making assumptions about it and needs to take steps.

PS: the new storage solution will not suffer from this but even next milestone, all UI data will still go into local storage just in case.

[jrieken]

Given that, no workspace storage is found anymore

Really? We have lost workspace storage and no one complained yet? Surprising...

[jrieken]

please advise what I can do here to get back the old behaviour.

The general advise is to never use the string "as-is" but to go via Uri.parse(input) and then uri.toString(). That will generate the 'new' representation which is safe to be used

[bpasero]

Really? We have lost workspace storage and no one complained yet? Surprising...

Well, windows only and insiders only and its very subtle, visually you just do not see the opened editors anymore. And its "fixed" once you continue to use that state because then you use the new form of the key.

The general advise is to never use the string "as-is" but to go via Uri.parse(input) and then uri.toString(). That will generate the 'new' representation which is safe to be used

In order to access an entry in local storage I need its full key (storage.getItem(key)). That key uses %3A for the drive letter. I do not have a full model of local storage in memory for the workspace (only the entire contents across all workspaces). I would prefer a way where I do not need to convert all keys in local storage to use the "safe" form if possible.

[jrieken]

Hm, I see... Yeah, we can add an overload to toString that restores the old behaviour then. That should be the easiest.

[jrieken]

@bpasero I have pushed a change that adds another argument to the URI#toString: https://github.com/Microsoft/vscode/blob/c537eaa14796b8e6be21b68d8ea5084e199cdb0e/src/vs/base/common/uri.ts#L349

[jrieken]

Alternatively (maybe better), this regex can be used: file:\/\/\/([a-zA-Z]): and then replace $1 with %3A

[jrieken]

I like the regex option better because it's more local. I will revert above commit changes

[bpasero]

@jrieken I am fine using a regex in my case since that code will be thrown away at one point anyway.

I feel like we should maybe ping everyone and give some advice given this change has the potential of breaking bad if used in the wrong way. For example I see URIs stored in the old format (as toString()) in storage.json (e.g. list of recently opened folders on the main side) and backup service (/cc @aeschli).

[jrieken]

Hm, that's what I was afraid of. we can also think about undoing the root case again...

[bpasero]

@jrieken let's discuss in the office

[jrieken]

I have reverted the uri changes that I have done for #58985.