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

Linkify variable values in repl; #79198 #80502

Merged
merged 1 commit into from
Sep 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 20 additions & 8 deletions src/vs/workbench/contrib/debug/browser/baseDebugView.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -29,6 +30,7 @@ export interface IRenderValueOptions {
maxValueLength?: number;
showHover?: boolean;
colorize?: boolean;
linkDetector?: LinkDetector;
}

export interface IVariableTemplateData {
Expand Down Expand Up @@ -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') {
Expand All @@ -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
});
}

Expand Down Expand Up @@ -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);
Expand Down
12 changes: 8 additions & 4 deletions src/vs/workbench/contrib/debug/browser/linkDetector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -153,6 +157,6 @@ export class LinkDetector {
startColumn: column
}
}
}, group);
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,16 @@
margin-left: 6px;
}

/* Links */
Copy link
Contributor

Choose a reason for hiding this comment

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

I think changes in this file are no longer necessery. Especially for the debug hover.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The .expression .value a is still needed for everything rendered by renderExpressionValue. Removed the debug hover part. Actually, these generic ones also work for repl, so I removed extra css from repl.css.


.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 {
Expand Down
7 changes: 0 additions & 7 deletions src/vs/workbench/contrib/debug/browser/media/repl.css
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
61 changes: 44 additions & 17 deletions src/vs/workbench/contrib/debug/browser/repl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.$;

Expand Down Expand Up @@ -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>,
Expand Down Expand Up @@ -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'));
Expand All @@ -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';
Expand All @@ -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');
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down
34 changes: 30 additions & 4 deletions src/vs/workbench/contrib/debug/test/browser/baseDebugView.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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"');

Expand All @@ -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', () => {
Expand All @@ -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');
Expand Down