Skip to content

Conversation

@samrg123
Copy link

Fixed issue #108301 by escaping URI with 'encodeURI' before creating a MarkdownString in 'TerminalLinkManager.ts::_getLinkHoverString'.

You can test the original bug and fix by creating an extension that registers a TerminalLinkProvider that returns a TerminalLink spanning a space.

The extension activate function I used for testing was:

export function activate(context: vscode.ExtensionContext) {

	// Use the console to output diagnostic information (console.log) and errors (console.error)
	// This line of code will only be executed once when your extension is activated
	console.log('Congratulations, your extension "terminallinktest" is now active!');


	vscode.window.activeTerminal?.show();


	vscode.window.registerTerminalLinkProvider({
		provideTerminalLinks: (context, token) => {

			return [{
				startIndex: 0,
				length: 10,
				tooltip: "crash the tooltip"
			}];
		},

		handleTerminalLink: (link: any) => { }
	});

	// The command has been defined in the package.json file
	// Now provide the implementation of the command with registerCommand
	// The commandId parameter must match the command field in package.json
	let disposable = vscode.commands.registerCommand('terminallinktest.testLink', () => {
		// The code you place here will be executed every time your command is executed

		vscode.window.showInformationMessage("Hello from termainLinkTest");

		// const crashPrefix = "package:test_api/src/frontend/expect.dart";
		// const crashMiddle = " 154:30";
		// const crashSuffix = "		fail";
		// const crashString = crashPrefix + crashMiddle + crashSuffix;

		let text = "";

		// Send all ascii chars
		for(var i = 32; i < 300; ++i) {

			let message = i+":";

			// escape quotes and ticks
			if(i === 34 || i === 96) {
				message = "'"+message+"\\"+String.fromCharCode(i)+"'";

			// escape "'"
			} else if(i === 39) {

				message = '"'+message+'\'"';

			// default
			} else {
				message = "'"+message+String.fromCharCode(i)+"'";
			}

			text+= message+" ";
		}

		vscode.window.activeTerminal?.sendText("echo "+text);
	});

	context.subscriptions.push(disposable);
}

This PR fixes #108301

Copy link

@rheh rheh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One suggestion - other than that code looks good.

Copy link

@rheh rheh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect

@Tyriar Tyriar added this to the February 2021 milestone Jan 20, 2021
@Tyriar
Copy link
Member

Tyriar commented Feb 5, 2021

Sorry for not getting to this, it looks like someone on the team got to it in the meantime in #110509

@Tyriar Tyriar closed this Feb 5, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Mar 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Terminal tooltips render markdown if they span a range that includes colons

4 participants