Mismatching normalized URI in plugin system

[colin-grant-work]

Bug Description:

This plugin uses the following sequence of calls to open, reveal, and set the selection in an editor:

https://github.com/alefragnani/vscode-bookmarks/blob/83483c19da24c2246436592b262f871f1cd39a17/src/extension.ts#L205-L213

For whatever reason - likely to do with workspace handling - the URI sometimes contains a double slash, e.g.

file:///home/colin/code/open/theia//packages/monaco/src/browser/monaco-editor-provider.ts
                                  ^^

This gets through the openTextDocument call fine, but in showTextDocument we have this code:

theia/packages/plugin-ext/src/plugin/plugin-context.ts

Lines 385 to 391 in f4409b0

	await documents.showDocument(uri, documentOptions);
	const textEditor = editors.getVisibleTextEditors().find(editor => editor.document.uri.toString() === uri.toString());
	if (textEditor) {
	return Promise.resolve(textEditor);
	} else {
	throw new Error(`Failed to show text document ${documentArg.toString()}`);
	}

The URI that is given by editor.document.uri.toString() is normalized to something like

file:///home/colin/code/open/theia/packages/monaco/src/browser/monaco-editor-provider.ts
                                  ^

with no double slash, so the .find call returns undefined, and we throw an error in line 390, even though the document has actually been shown.

Steps to Reproduce:

1. Install this plugin uri-normalization-problems-0.0.1.zip. Source here.
2. Open Theia with Theia as workspace. Close any existing editors.
3. Run the command 'VSCode API: Open document and reveal range'
4. Note that the file is opened, but you are at the top of the file

Additional Information

• Operating System: Ubuntu 20, RHEL7
• Theia Version: master

[colin-grant-work]

@msujew, do you have any thoughts here? Is the mistake allowing the ill-formed URI in the first place (and we should modify our URI implementations to do some normalization when the URI is created), or is the mistake the fact that we do normalize at some point - probably resource resolution - and so depart from the input we've received? VSCode recently added a UriIdentityService to handle the kinds of comparisons we're currently doing in the showTextDocument find, as another approach.

[msujew]

@colin-grant-work I guess it's a combination of both. Something that irked me quite for a while already was the fact that we effectively use two different URI/path implementations. See for example:

const uri = new URI('file:///tmp//doubleslash');
const vscodeUriString = uri.toString(); // this will result in file:///tmp//doubleslash'
const theiaUriString = uri.schema + uri.path.toString(); // this will result in 'file:///tmp/doubleslash'

Note the missing doubleslash in the theiaUriString, since the Theia URI path is normalized, but the path of the vscode URI isn't.

We could go the same way as vscode and implement such an identity service. Alternatively, we could feed the normalized path back into the vscode URI after parsing it from a string, but I'm not sure how safe this is, considering we have parts of the app (plugin side) which almost exclusively deals in vscode URIs.

[colin-grant-work]

const uri = new URI('file:///tmp//doubleslash');
const vscodeUriString = uri.toString(); // this will result in file:///tmp//doubleslash'
const theiaUriString = uri.schema + uri.path.toString(); // this will result in 'file:///tmp/doubleslash'

Yikes - that's no good at all!

[msujew]

@colin-grant-work Nvm, I believe it does only do that if we actually manually call Path.normalize(), but there's still a normalization step when constructing paths:

theia/packages/core/src/common/path.ts

Lines 145 to 146 in d8a6ab4

	raw = Path.normalizePathSeparator(raw);
	this.raw = Path.normalizeDrive(raw);