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

Make sure markdown links execute an ext-host vscode.open command always #154993

Closed
mjbvz opened this issue Jul 12, 2022 · 9 comments
Closed

Make sure markdown links execute an ext-host vscode.open command always #154993

mjbvz opened this issue Jul 12, 2022 · 9 comments
Assignees
Labels
*dev-question VS Code Extension Development Question markdown Markdown support issues

Comments

@mjbvz
Copy link
Collaborator

mjbvz commented Jul 12, 2022

I'm trying to use vscode.open with DocumentLinkProvider. At present, only the first argument of vscode.open (the resource) seems to be respected. The other arguments are ignored.

Here's an example extension that demonstrates this issue:

import * as vscode from 'vscode';

export function activate(context: vscode.ExtensionContext) {
	// Open `abc.md` from workspace root and select a range in it
	const commandUriWithOptions = vscode.Uri.parse(
		createCommandUri('vscode.open', vscode.Uri.joinPath(vscode.workspace.workspaceFolders![0].uri, 'abc.md'), {
			selection: new vscode.Range(2, 0, 5, 0),
		})
	);

	// Open `abc.md` from workspace root besides the current file
	const commandUriWithViewCol = vscode.Uri.parse(
		createCommandUri('vscode.open', vscode.Uri.joinPath(vscode.workspace.workspaceFolders![0].uri, 'abc.md'), vscode.ViewColumn.Beside)
	);

	// Make sure basic command run on ext host side works
	//vscode.commands.executeCommand('vscode.open', vscode.Uri.joinPath(vscode.workspace.workspaceFolders![0].uri, 'abc.md'), vscode.ViewColumn.Beside);

	vscode.languages.registerDocumentLinkProvider('markdown', {
		provideDocumentLinks: (doc) => {
			return [
				new vscode.DocumentLink(new vscode.Range(0, 0, 100, 0), commandUriWithViewCol)
			];
		}
	});
}

function createCommandUri(command: string, ...args: any[]): string {
	return `command:${command}?${encodeURIComponent(JSON.stringify(args))}`;
}

To repo:

  1. Create abc.md in the root of your workspace. Add a few lines of content to it
  2. Open another md file and open it
  3. Click on the link from the provider

Expected

This should open abc.md besides the current editor

Actual
abc.md is opened in the same view column

@mjbvz
Copy link
Collaborator Author

mjbvz commented Jul 12, 2022

I believe the root cause is that this ends invoking the renderer version of vscode.open:

CommandsRegistry.registerCommand({
id: 'vscode.open',
handler: (accessor, arg) => {
accessor.get(ICommandService).executeCommand(API_OPEN_EDITOR_COMMAND_ID, arg);
},
description: {
description: 'Opens the provided resource in the editor.',
args: [{ name: 'Uri' }]
}
});

Instead of the one on the extension host that supports all the additional arguments

@mjbvz
Copy link
Collaborator Author

mjbvz commented Jul 12, 2022

I also tried using _workbench.open directly. That doesn't work well since we don't go through the ext host type conversions. This means that the selection argument in the first example doesn't work since it has the wrong shape

@jrieken
Copy link
Member

jrieken commented Jul 13, 2022

This needs special handling, like done for tree views etc, but that also means extensions cannot (and should not) be able to control the opening details but that the user-gesture (ctrl+click vs normal click) determines the "to the side" behaviour

@bpasero
Copy link
Member

bpasero commented Jul 13, 2022

Took me a while to understand. First of all, vscode.open is clearly supporting an argument to open to the side:

new ApiCommand(
'vscode.open', '_workbench.open', 'Opens the provided resource in the editor. Can be a text or binary file, or an http(s) URL. If you need more control over the options for opening a text file, use vscode.window.showTextDocument instead.',
[
new ApiCommandArgument<URI | string>('uriOrString', 'Uri-instance or string (only http/https)', v => URI.isUri(v) || (typeof v === 'string' && matchesSomeScheme(v, Schemas.http, Schemas.https)), v => v),
new ApiCommandArgument<vscode.ViewColumn | typeConverters.TextEditorOpenOptions | undefined, [vscode.ViewColumn?, ITextEditorOptions?] | undefined>('columnOrOptions', 'Either the column in which to open or editor options, see vscode.TextDocumentShowOptions',
v => v === undefined || typeof v === 'number' || typeof v === 'object',
v => !v ? v : typeof v === 'number' ? [typeConverters.ViewColumn.from(v), undefined] : [typeConverters.ViewColumn.from(v.viewColumn), typeConverters.TextEditorOpenOptions.from(v)]
).optional(),
ApiCommandArgument.String.with('label', '').optional()
],
ApiCommandResult.Void
),

This particular issue seems to be around command links in a document (which I am not aware of how they work). If I understand correctly, these links execute on the renderer and not on the extension host and as such, the additional arguments are not supported:

// partial, renderer-side API command to open editor
// complements https://github.com/microsoft/vscode/blob/2b164efb0e6a5de3826bff62683eaeafe032284f/src/vs/workbench/api/common/extHostApiCommands.ts#L373
CommandsRegistry.registerCommand({
id: 'vscode.open',
handler: (accessor, arg) => {
accessor.get(ICommandService).executeCommand(API_OPEN_EDITOR_COMMAND_ID, arg);
},
description: {
description: 'Opens the provided resource in the editor.',
args: [{ name: 'Uri' }]
}
});

Why can the renderer side not simply allow for the same options?

@bpasero bpasero added info-needed Issue requires more information from poster api workbench-editors Managing of editor widgets in workbench window and removed info-needed Issue requires more information from poster labels Jul 13, 2022
@jrieken
Copy link
Member

jrieken commented Jul 13, 2022

Why can the renderer side not simply allow for the same options?

Because those are API objects and no one transforms them into internal objects

@bpasero
Copy link
Member

bpasero commented Jul 13, 2022

but that the user-gesture (ctrl+click vs normal click) determines the "to the side" behaviour

It seems to me that Matt wants to always open to the side, irrespective to the user-gesture?

If so, I believe this is a feature request for document links to:

  • either execute from the ext host side
  • or to add simple arguments to the renderer side for this particular scenario that does not drag in API types

@bpasero
Copy link
Member

bpasero commented Jul 22, 2022

Let's maybe sync the three of us in August.

@bpasero bpasero added this to the August 2022 milestone Jul 22, 2022
@bpasero bpasero changed the title vscode.open on document link doesn't accept additional arguments vscode.open on document link doesn't accept additional arguments Jul 22, 2022
@bpasero bpasero added the feature-request Request for new features or functionality label Aug 19, 2022
@bpasero bpasero modified the milestones: August 2022, September 2022 Aug 19, 2022
@bpasero bpasero added markdown Markdown support issues and removed feature-request Request for new features or functionality api workbench-editors Managing of editor widgets in workbench window labels Sep 7, 2022
@bpasero bpasero changed the title vscode.open on document link doesn't accept additional arguments Make sure markdown links execute an ext-host vscode.open command always Sep 7, 2022
@bpasero
Copy link
Member

bpasero commented Sep 7, 2022

As discussed, the suggested fix is to call a vscode.open clone that ensures ext-host side execution and argument conversion.

@mjbvz
Copy link
Collaborator Author

mjbvz commented Sep 8, 2022

Fixed by 077f586

@mjbvz mjbvz closed this as completed Sep 8, 2022
@mjbvz mjbvz added the *dev-question VS Code Extension Development Question label Sep 8, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Oct 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
*dev-question VS Code Extension Development Question markdown Markdown support issues
Projects
None yet
Development

No branches or pull requests

3 participants