Merge pull request #80502 from dgozman/fix-79198

Linkify variable values in repl; #79198
microsoft · Sep 13, 2019 · 9b0b596 · 9b0b596
2 parents 4f39995 + c526947
commit 9b0b596
Show file tree

Hide file tree

Showing 6 changed files with 112 additions and 40 deletions.
diff --git a/src/vs/workbench/contrib/debug/browser/baseDebugView.ts b/src/vs/workbench/contrib/debug/browser/baseDebugView.ts
@@ -16,6 +16,7 @@ import { KeyCode } from 'vs/base/common/keyCodes';
 import { IKeyboardEvent } from 'vs/base/browser/keyboardEvent';
 import { HighlightedLabel, IHighlight } from 'vs/base/browser/ui/highlightedlabel/highlightedLabel';
 import { FuzzyScore, createMatches } from 'vs/base/common/filters';
+import { LinkDetector } from 'vs/workbench/contrib/debug/browser/linkDetector';
 
 export const MAX_VALUE_RENDER_LENGTH_IN_VIEWLET = 1024;
 export const twistiePixels = 20;
@@ -29,6 +30,7 @@ export interface IRenderValueOptions {
 	maxValueLength?: number;
 	showHover?: boolean;
 	colorize?: boolean;
+	linkDetector?: LinkDetector;
 }
 
 export interface IVariableTemplateData {
@@ -83,16 +85,22 @@ export function renderExpressionValue(expressionOrValue: IExpressionContainer |
 		value = value.substr(0, options.maxValueLength) + '...';
 	}
 	if (value && !options.preserveWhitespace) {
-		container.textContent = replaceWhitespace(value);
+		value = replaceWhitespace(value);
 	} else {
-		container.textContent = value || '';
+		value = value || '';
+	}
+	if (options.linkDetector) {
+		container.textContent = '';
+		container.appendChild(options.linkDetector.handleLinks(value));
+	} else {
+		container.textContent = value;
 	}
 	if (options.showHover) {
 		container.title = value || '';
 	}
 }
 
-export function renderVariable(variable: Variable, data: IVariableTemplateData, showChanged: boolean, highlights: IHighlight[]): void {
+export function renderVariable(variable: Variable, data: IVariableTemplateData, showChanged: boolean, highlights: IHighlight[], linkDetector?: LinkDetector): void {
 	if (variable.available) {
 		let text = replaceWhitespace(variable.name);
 		if (variable.value && typeof variable.name === 'string') {
@@ -109,7 +117,8 @@ export function renderVariable(variable: Variable, data: IVariableTemplateData,
 		maxValueLength: MAX_VALUE_RENDER_LENGTH_IN_VIEWLET,
 		preserveWhitespace: false,
 		showHover: true,
-		colorize: true
+		colorize: true,
+		linkDetector
 	});
 }
 
@@ -209,14 +218,17 @@ export abstract class AbstractExpressionsRenderer implements ITreeRenderer<IExpr
 	renderElement(node: ITreeNode<IExpression, FuzzyScore>, index: number, data: IExpressionTemplateData): void {
 		const { element } = node;
 		if (element === this.debugService.getViewModel().getSelectedExpression()) {
-			data.enableInputBox(element, this.getInputBoxOptions(element));
-		} else {
-			this.renderExpression(element, data, createMatches(node.filterData));
+			const options = this.getInputBoxOptions(element);
+			if (options) {
+				data.enableInputBox(element, options);
+				return;
+			}
 		}
+		this.renderExpression(element, data, createMatches(node.filterData));
 	}
 
 	protected abstract renderExpression(expression: IExpression, data: IExpressionTemplateData, highlights: IHighlight[]): void;
-	protected abstract getInputBoxOptions(expression: IExpression): IInputBoxOptions;
+	protected abstract getInputBoxOptions(expression: IExpression): IInputBoxOptions | undefined;
 
 	disposeTemplate(templateData: IExpressionTemplateData): void {
 		dispose(templateData.toDispose);

diff --git a/src/vs/workbench/contrib/debug/browser/linkDetector.ts b/src/vs/workbench/contrib/debug/browser/linkDetector.ts
@@ -9,7 +9,7 @@ import { URI as uri } from 'vs/base/common/uri';
 import { isMacintosh } from 'vs/base/common/platform';
 import { IMouseEvent, StandardMouseEvent } from 'vs/base/browser/mouseEvent';
 import * as nls from 'vs/nls';
-import { IEditorService, SIDE_GROUP, ACTIVE_GROUP } from 'vs/workbench/services/editor/common/editorService';
+import { IEditorService } from 'vs/workbench/services/editor/common/editorService';
 
 export class LinkDetector {
 	private static readonly MAX_LENGTH = 500;
@@ -87,11 +87,13 @@ export class LinkDetector {
 
 					const link = document.createElement('a');
 					link.textContent = line.substr(match.index, match[0].length);
-					link.title = isMacintosh ? nls.localize('fileLinkMac', "Click to follow (Cmd + click opens to the side)") : nls.localize('fileLink', "Click to follow (Ctrl + click opens to the side)");
+					link.title = isMacintosh ? nls.localize('fileLinkMac', "Cmd + click to follow link") : nls.localize('fileLink', "Ctrl + click to follow link");
 					lineContainer.appendChild(link);
 					const lineNumber = Number(match[3]);
 					const columnNumber = match[4] ? Number(match[4]) : undefined;
 					link.onclick = (e) => this.onLinkClick(new StandardMouseEvent(e), resource!, lineNumber, columnNumber);
+					link.onmousemove = (event) => link.classList.toggle('pointer', isMacintosh ? event.metaKey : event.ctrlKey);
+					link.onmouseleave = () => link.classList.remove('pointer');
 
 					lastMatchIndex = pattern.lastIndex;
 					const currentMatch = match;
@@ -141,9 +143,11 @@ export class LinkDetector {
 		if (!selection || selection.type === 'Range') {
 			return; // do not navigate when user is selecting
 		}
+		if (!(isMacintosh ? event.metaKey : event.ctrlKey)) {
+			return;
+		}
 
 		event.preventDefault();
-		const group = event.ctrlKey || event.metaKey ? SIDE_GROUP : ACTIVE_GROUP;
 
 		this.editorService.openEditor({
 			resource,
@@ -153,6 +157,6 @@ export class LinkDetector {
 					startColumn: column
 				}
 			}
-		}, group);
+		});
 	}
 }
diff --git a/src/vs/workbench/contrib/debug/browser/media/debug.contribution.css b/src/vs/workbench/contrib/debug/browser/media/debug.contribution.css
@@ -147,6 +147,16 @@
 	margin-left: 6px;
 }
 
+/* Links */
+
+.monaco-workbench .monaco-list-row .expression .value a {
+	text-decoration: underline;
+}
+
+.monaco-workbench .monaco-list-row .expression .value a.pointer {
+	cursor: pointer;
+}
+
 /* White color when element is selected and list is focused. White looks better on blue selection background. */
 .monaco-workbench .monaco-list:focus .monaco-list-row.selected .expression .name,
 .monaco-workbench .monaco-list:focus .monaco-list-row.selected .expression .value {

diff --git a/src/vs/workbench/contrib/debug/browser/media/repl.css b/src/vs/workbench/contrib/debug/browser/media/repl.css
@@ -134,10 +134,3 @@
 .monaco-workbench .repl .repl-tree .output.expression .code-bold	{ font-weight: bold; }
 .monaco-workbench .repl .repl-tree .output.expression .code-italic	{ font-style: italic; }
 .monaco-workbench .repl .repl-tree .output.expression .code-underline { text-decoration: underline; }
-
-/* Links */
-.monaco-workbench .repl .repl-tree .output.expression a,
-.monaco-workbench .repl .repl-tree .evaluation-result.expression a {
-	text-decoration: underline;
-	cursor: pointer;
-}
diff --git a/src/vs/workbench/contrib/debug/browser/repl.ts b/src/vs/workbench/contrib/debug/browser/repl.ts
@@ -49,21 +49,20 @@ import { SimpleReplElement, RawObjectReplElement, ReplEvaluationInput, ReplEvalu
 import { IListVirtualDelegate } from 'vs/base/browser/ui/list/list';
 import { ITreeRenderer, ITreeNode, ITreeContextMenuEvent, IAsyncDataSource } from 'vs/base/browser/ui/tree/tree';
 import { IEditorService } from 'vs/workbench/services/editor/common/editorService';
-import { renderExpressionValue } from 'vs/workbench/contrib/debug/browser/baseDebugView';
+import { renderExpressionValue, AbstractExpressionsRenderer, IExpressionTemplateData, renderVariable, IInputBoxOptions } from 'vs/workbench/contrib/debug/browser/baseDebugView';
 import { handleANSIOutput } from 'vs/workbench/contrib/debug/browser/debugANSIHandling';
 import { ILabelService } from 'vs/platform/label/common/label';
 import { LinkDetector } from 'vs/workbench/contrib/debug/browser/linkDetector';
 import { Separator } from 'vs/base/browser/ui/actionbar/actionbar';
-import { IContextMenuService } from 'vs/platform/contextview/browser/contextView';
+import { IContextMenuService, IContextViewService } from 'vs/platform/contextview/browser/contextView';
 import { removeAnsiEscapeCodes } from 'vs/base/common/strings';
 import { WorkbenchAsyncDataTree } from 'vs/platform/list/browser/listService';
 import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
 import { ITextResourcePropertiesService } from 'vs/editor/common/services/resourceConfiguration';
 import { RunOnceScheduler } from 'vs/base/common/async';
 import { FuzzyScore, createMatches } from 'vs/base/common/filters';
-import { HighlightedLabel } from 'vs/base/browser/ui/highlightedlabel/highlightedLabel';
+import { HighlightedLabel, IHighlight } from 'vs/base/browser/ui/highlightedlabel/highlightedLabel';
 import { IClipboardService } from 'vs/platform/clipboard/common/clipboardService';
-import { VariablesRenderer } from 'vs/workbench/contrib/debug/browser/variablesView';
 
 const $ = dom.$;
 
@@ -405,17 +404,18 @@ export class Repl extends Panel implements IPrivateReplService, IHistoryNavigati
 		this.replDelegate = new ReplDelegate(this.configurationService);
 		const wordWrap = this.configurationService.getValue<IDebugConfiguration>('debug').console.wordWrap;
 		dom.toggleClass(treeContainer, 'word-wrap', wordWrap);
+		const linkDetector = this.instantiationService.createInstance(LinkDetector);
 		this.tree = this.instantiationService.createInstance(
 			WorkbenchAsyncDataTree,
 			'DebugRepl',
 			treeContainer,
 			this.replDelegate,
 			[
-				this.instantiationService.createInstance(VariablesRenderer),
-				this.instantiationService.createInstance(ReplSimpleElementsRenderer),
+				this.instantiationService.createInstance(ReplVariablesRenderer, linkDetector),
+				this.instantiationService.createInstance(ReplSimpleElementsRenderer, linkDetector),
 				new ReplEvaluationInputsRenderer(),
-				new ReplEvaluationResultsRenderer(),
-				new ReplRawObjectsRenderer()
+				new ReplEvaluationResultsRenderer(linkDetector),
+				new ReplRawObjectsRenderer(linkDetector),
 			],
 			// https://github.com/microsoft/TypeScript/issues/32526
 			new ReplDataSource() as IAsyncDataSource<IDebugSession, IReplElement>,
@@ -626,6 +626,8 @@ class ReplEvaluationResultsRenderer implements ITreeRenderer<ReplEvaluationResul
 		return ReplEvaluationResultsRenderer.ID;
 	}
 
+	constructor(private readonly linkDetector: LinkDetector) { }
+
 	renderTemplate(container: HTMLElement): IReplEvaluationResultTemplateData {
 		const output = dom.append(container, $('.evaluation-result.expression'));
 		const value = dom.append(output, $('span.value'));
@@ -639,7 +641,8 @@ class ReplEvaluationResultsRenderer implements ITreeRenderer<ReplEvaluationResul
 		renderExpressionValue(expression, templateData.value, {
 			preserveWhitespace: !expression.hasChildren,
 			showHover: false,
-			colorize: true
+			colorize: true,
+			linkDetector: this.linkDetector
 		});
 		if (expression.hasChildren) {
 			templateData.annotation.className = 'annotation octicon octicon-info';
@@ -656,21 +659,16 @@ class ReplSimpleElementsRenderer implements ITreeRenderer<SimpleReplElement, Fuz
 	static readonly ID = 'simpleReplElement';
 
 	constructor(
+		private readonly linkDetector: LinkDetector,
 		@IEditorService private readonly editorService: IEditorService,
 		@ILabelService private readonly labelService: ILabelService,
-		@IInstantiationService private readonly instantiationService: IInstantiationService,
 		@IThemeService private readonly themeService: IThemeService
 	) { }
 
 	get templateId(): string {
 		return ReplSimpleElementsRenderer.ID;
 	}
 
-	@memoize
-	get linkDetector(): LinkDetector {
-		return this.instantiationService.createInstance(LinkDetector);
-	}
-
 	renderTemplate(container: HTMLElement): ISimpleReplElementTemplateData {
 		const data: ISimpleReplElementTemplateData = Object.create(null);
 		dom.addClass(container, 'output');
@@ -716,9 +714,37 @@ class ReplSimpleElementsRenderer implements ITreeRenderer<SimpleReplElement, Fuz
 	}
 }
 
+export class ReplVariablesRenderer extends AbstractExpressionsRenderer {
+
+	static readonly ID = 'replVariable';
+
+	get templateId(): string {
+		return ReplVariablesRenderer.ID;
+	}
+
+	constructor(
+		private readonly linkDetector: LinkDetector,
+		@IDebugService debugService: IDebugService,
+		@IContextViewService contextViewService: IContextViewService,
+		@IThemeService themeService: IThemeService,
+	) {
+		super(debugService, contextViewService, themeService);
+	}
+
+	protected renderExpression(expression: IExpression, data: IExpressionTemplateData, highlights: IHighlight[]): void {
+		renderVariable(expression as Variable, data, true, highlights, this.linkDetector);
+	}
+
+	protected getInputBoxOptions(expression: IExpression): IInputBoxOptions | undefined {
+		return undefined;
+	}
+}
+
 class ReplRawObjectsRenderer implements ITreeRenderer<RawObjectReplElement, FuzzyScore, IRawObjectReplTemplateData> {
 	static readonly ID = 'rawObject';
 
+	constructor(private readonly linkDetector: LinkDetector) { }
+
 	get templateId(): string {
 		return ReplRawObjectsRenderer.ID;
 	}
@@ -748,7 +774,8 @@ class ReplRawObjectsRenderer implements ITreeRenderer<RawObjectReplElement, Fuzz
 		// value
 		renderExpressionValue(element.value, templateData.value, {
 			preserveWhitespace: true,
-			showHover: false
+			showHover: false,
+			linkDetector: this.linkDetector
 		});
 
 		// annotation if any
@@ -807,7 +834,7 @@ class ReplDelegate implements IListVirtualDelegate<IReplElement> {
 
 	getTemplateId(element: IReplElement): string {
 		if (element instanceof Variable && element.name) {
-			return VariablesRenderer.ID;
+			return ReplVariablesRenderer.ID;
 		}
 		if (element instanceof ReplEvaluationResult) {
 			return ReplEvaluationResultsRenderer.ID;

diff --git a/src/vs/workbench/contrib/debug/test/browser/baseDebugView.test.ts b/src/vs/workbench/contrib/debug/test/browser/baseDebugView.test.ts
@@ -9,9 +9,21 @@ import * as dom from 'vs/base/browser/dom';
 import { Expression, Variable, Scope, StackFrame, Thread } from 'vs/workbench/contrib/debug/common/debugModel';
 import { MockSession } from 'vs/workbench/contrib/debug/test/common/mockDebug';
 import { HighlightedLabel } from 'vs/base/browser/ui/highlightedlabel/highlightedLabel';
+import { LinkDetector } from 'vs/workbench/contrib/debug/browser/linkDetector';
+import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock';
+import { workbenchInstantiationService } from 'vs/workbench/test/workbenchTestServices';
 const $ = dom.$;
 
 suite('Debug - Base Debug View', () => {
+	let linkDetector: LinkDetector;
+
+	/**
+	 * Instantiate services for use by the functions being tested.
+	 */
+	setup(() => {
+		const instantiationService: TestInstantiationService = <TestInstantiationService>workbenchInstantiationService();
+		linkDetector = instantiationService.createInstance(LinkDetector);
+	});
 
 	test('replace whitespace', () => {
 		assert.equal(replaceWhitespace('hey there'), 'hey there');
@@ -36,7 +48,7 @@ suite('Debug - Base Debug View', () => {
 		expression.available = true;
 		expression.value = '"string value"';
 		container = $('.container');
-		renderExpressionValue(expression, container, { colorize: true });
+		renderExpressionValue(expression, container, { colorize: true, linkDetector });
 		assert.equal(container.className, 'value string');
 		assert.equal(container.textContent, '"string value"');
 
@@ -48,8 +60,14 @@ suite('Debug - Base Debug View', () => {
 
 		expression.value = 'this is a long string';
 		container = $('.container');
-		renderExpressionValue(expression, container, { colorize: true, maxValueLength: 4 });
+		renderExpressionValue(expression, container, { colorize: true, maxValueLength: 4, linkDetector });
 		assert.equal(container.textContent, 'this...');
+
+		expression.value = process.platform === 'win32' ? 'C:\\foo.js:5' : '/foo.js:5';
+		container = $('.container');
+		renderExpressionValue(expression, container, { colorize: true, linkDetector });
+		assert.ok(container.querySelector('a'));
+		assert.equal(container.querySelector('a')!.textContent, expression.value);
 	});
 
 	test('render variable', () => {
@@ -73,16 +91,24 @@ suite('Debug - Base Debug View', () => {
 		expression = $('.');
 		name = $('.');
 		value = $('.');
-		renderVariable(variable, { expression, name, value, label }, false, []);
+		renderVariable(variable, { expression, name, value, label }, false, [], linkDetector);
 		assert.equal(value.textContent, 'hey');
 		assert.equal(label.element.textContent, 'foo:');
 		assert.equal(label.element.title, 'string');
 
+		variable.value = process.platform === 'win32' ? 'C:\\foo.js:5' : '/foo.js:5';
+		expression = $('.');
+		name = $('.');
+		value = $('.');
+		renderVariable(variable, { expression, name, value, label }, false, [], linkDetector);
+		assert.ok(value.querySelector('a'));
+		assert.equal(value.querySelector('a')!.textContent, variable.value);
+
 		variable = new Variable(session, scope, 2, 'console', 'console', '5', 0, 0, { kind: 'virtual' });
 		expression = $('.');
 		name = $('.');
 		value = $('.');
-		renderVariable(variable, { expression, name, value, label }, false, []);
+		renderVariable(variable, { expression, name, value, label }, false, [], linkDetector);
 		assert.equal(name.className, 'virtual');
 		assert.equal(label.element.textContent, 'console:');
 		assert.equal(label.element.title, 'console');