Skip to content

Commit

Permalink
Merge pull request #1295 from microsoft/feat/151410
Browse files Browse the repository at this point in the history
feat: simplify pretty print to align with devtools
  • Loading branch information
connor4312 authored Jun 7, 2022
2 parents 1a57656 + 3eeb048 commit d816d6a
Show file tree
Hide file tree
Showing 16 changed files with 101 additions and 269 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ This changelog records changes to stable releases since 1.50.2. "TBA" changes he

## Nightly (only)

- feat: simplify pretty print to align with devtools ([vscode#151410](https://github.com/microsoft/vscode/issues/151410))
- fix: debugged child processes in ext host causing teardown ([#1289](https://github.com/microsoft/vscode-js-debug/issues/1289))
- fix: errors thrown in process tree lookup not being visible ([vscode#150754](https://github.com/microsoft/vscode/issues/150754))
- chore: adopt new restartFrame semantics from Chrome 104 ([#1283](https://github.com/microsoft/vscode-js-debug/issues/1283))
Expand Down
15 changes: 0 additions & 15 deletions src/adapter/debugAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,6 @@ export class DebugAdapter implements IDisposable {
this.dap.on('enableCustomBreakpoints', params => this.enableCustomBreakpoints(params));
this.dap.on('toggleSkipFileStatus', params => this._toggleSkipFileStatus(params));
this.dap.on('disableCustomBreakpoints', params => this._disableCustomBreakpoints(params));
this.dap.on('canPrettyPrintSource', params => this._canPrettyPrintSource(params));
this.dap.on('prettyPrintSource', params => this._prettyPrintSource(params));
this.dap.on('revealPage', () => this._withThread(thread => thread.revealPage()));
this.dap.on('getPerformance', () =>
Expand Down Expand Up @@ -438,20 +437,6 @@ export class DebugAdapter implements IDisposable {
return {};
}

async _canPrettyPrintSource(
params: Dap.CanPrettyPrintSourceParams,
): Promise<Dap.CanPrettyPrintSourceResult | Dap.Error> {
if (!params.source) {
return { canPrettyPrint: false };
}

params.source.path = urlUtils.platformPathToPreferredCase(params.source.path);
const source = this.sourceContainer.source(params.source);
if (!source)
return errors.createSilentError(localize('error.sourceNotFound', 'Source not found'));
return { canPrettyPrint: source.canPrettyPrint() };
}

async _prettyPrintSource(
params: Dap.PrettyPrintSourceParams,
): Promise<Dap.PrettyPrintSourceResult | Dap.Error> {
Expand Down
9 changes: 1 addition & 8 deletions src/adapter/sources.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,20 +223,13 @@ export class Source {
return 'text/javascript';
}

/**
* Gets whether this source is able to be pretty-printed.
*/
public canPrettyPrint(): boolean {
return this._container && !this._name.endsWith('-pretty.js');
}

/**
* Pretty-prints the source. Generates a beauitified source map if possible
* and it hasn't already been done, and returns the created map and created
* ephemeral source. Returns undefined if the source can't be beautified.
*/
public async prettyPrint(): Promise<{ map: SourceMap; source: Source } | undefined> {
if (!this._container || !this.canPrettyPrint()) {
if (!this._container) {
return undefined;
}

Expand Down
23 changes: 0 additions & 23 deletions src/build/dapCustom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,29 +107,6 @@ const dapCustom: JSONSchema4 = {
required: ['ids'],
}),

...makeRequest(
'canPrettyPrintSource',
'Returns whether particular source can be pretty-printed.',
{
properties: {
source: {
$ref: '#/definitions/Source',
description: 'Source to be pretty printed.',
},
},
required: ['source'],
},
{
required: ['canPrettyPrint'],
properties: {
canPrettyPrint: {
type: 'boolean',
description: 'Whether source can be pretty printed.',
},
},
},
),

...makeRequest('prettyPrintSource', 'Pretty prints source for debugging.', {
properties: {
source: {
Expand Down
14 changes: 9 additions & 5 deletions src/build/generate-contributions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
AutoAttachMode,
Commands,
Configuration,
ContextKey,
Contributions,
CustomViews,
DebugType,
Expand Down Expand Up @@ -1110,11 +1111,6 @@ const configurationSchema: ConfigurationAttributes<IConfigurationTypes> = {
default: {},
properties: nodeTerminalConfiguration.configurationAttributes as { [key: string]: JSONSchema6 },
},
[Configuration.SuggestPrettyPrinting]: {
type: 'boolean',
description: refString('configuration.suggestPrettyPrinting'),
default: true,
},
[Configuration.AutoServerTunnelOpen]: {
type: 'boolean',
description: refString('configuration.automaticallyTunnelRemoteServer'),
Expand Down Expand Up @@ -1209,6 +1205,7 @@ const commands: ReadonlyArray<{
command: Commands.PrettyPrint,
title: refString('pretty.print.script'),
category: 'Debug',
icon: '$(json)',
},
{
command: Commands.ToggleSkipping,
Expand Down Expand Up @@ -1452,6 +1449,13 @@ const menus: Menus = {
when: `view == ${CustomViews.ExcludedCallers}`,
},
],
'editor/title': [
{
command: Commands.PrettyPrint,
group: 'navigation',
when: `resource in ${ContextKey.CanPrettyPrint}`,
},
],
};

const keybindings = [
Expand Down
2 changes: 0 additions & 2 deletions src/build/strings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,8 +280,6 @@ const strings = {
'Where a "Run" and "Debug" code lens should be shown in your npm scripts. It may be on "all", scripts, on "top" of the script section, or "never".',
'configuration.terminalOptions':
'Default launch options for the JavaScript debug terminal and npm scripts.',
'configuration.suggestPrettyPrinting':
'Whether to suggest pretty printing JavaScript code that looks minified when you step into it.',
'configuration.automaticallyTunnelRemoteServer':
'When debugging a remote web app, configures whether to automatically tunnel the remote server to your local machine.',
'configuration.debugByLinkOptions':
Expand Down
20 changes: 18 additions & 2 deletions src/common/contributionUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ export const enum Configuration {
TerminalDebugConfig = 'debug.javascript.terminalOptions',
PickAndAttachDebugOptions = 'debug.javascript.pickAndAttachOptions',
DebugByLinkOptions = 'debug.javascript.debugByLinkOptions',
SuggestPrettyPrinting = 'debug.javascript.suggestPrettyPrinting',
AutoServerTunnelOpen = 'debug.javascript.automaticallyTunnelRemoteServer',
AutoAttachMode = 'debug.javascript.autoAttachFilter',
AutoAttachSmartPatterns = 'debug.javascript.autoAttachSmartPattern',
Expand All @@ -153,7 +152,6 @@ export interface IConfigurationTypes {
[Configuration.NpmScriptLens]: 'all' | 'top' | 'never';
[Configuration.TerminalDebugConfig]: Partial<ITerminalLaunchConfiguration>;
[Configuration.PickAndAttachDebugOptions]: Partial<INodeAttachConfiguration>;
[Configuration.SuggestPrettyPrinting]: boolean;
[Configuration.AutoServerTunnelOpen]: boolean;
[Configuration.DebugByLinkOptions]:
| DebugByLinkState
Expand Down Expand Up @@ -256,3 +254,21 @@ export const writeConfig = <K extends keyof IConfigurationTypes>(
value: IConfigurationTypes[K],
target?: ConfigurationTarget,
) => wsp.getConfiguration().update(key, value, target);

export const enum ContextKey {
HasExcludedCallers = 'jsDebugHasExcludedCallers',
CanPrettyPrint = 'jsDebugCanPrettyPrint',
IsProfiling = 'jsDebugIsProfiling',
}

export interface IContextKeyTypes {
[ContextKey.HasExcludedCallers]: boolean;
[ContextKey.CanPrettyPrint]: string[];
[ContextKey.IsProfiling]: boolean;
}

export const setContextKey = async <K extends keyof IContextKeyTypes>(
ns: typeof commands,
key: K,
value: IContextKeyTypes[K] | null,
) => await ns.executeCommand('setContext', key, value);
33 changes: 0 additions & 33 deletions src/dap/api.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -842,20 +842,6 @@ export namespace Dap {
params: DisableCustomBreakpointsParams,
): Promise<DisableCustomBreakpointsResult>;

/**
* Returns whether particular source can be pretty-printed.
*/
on(
request: 'canPrettyPrintSource',
handler: (params: CanPrettyPrintSourceParams) => Promise<CanPrettyPrintSourceResult | Error>,
): () => void;
/**
* Returns whether particular source can be pretty-printed.
*/
canPrettyPrintSourceRequest(
params: CanPrettyPrintSourceParams,
): Promise<CanPrettyPrintSourceResult>;

/**
* Pretty prints source for debugging.
*/
Expand Down Expand Up @@ -1601,11 +1587,6 @@ export namespace Dap {
params: DisableCustomBreakpointsParams,
): Promise<DisableCustomBreakpointsResult>;

/**
* Returns whether particular source can be pretty-printed.
*/
canPrettyPrintSource(params: CanPrettyPrintSourceParams): Promise<CanPrettyPrintSourceResult>;

/**
* Pretty prints source for debugging.
*/
Expand Down Expand Up @@ -1881,20 +1862,6 @@ export namespace Dap {
breakpoints: BreakpointLocation[];
}

export interface CanPrettyPrintSourceParams {
/**
* Source to be pretty printed.
*/
source: Source;
}

export interface CanPrettyPrintSourceResult {
/**
* Whether source can be pretty printed.
*/
canPrettyPrint: boolean;
}

export interface CancelParams {
/**
* The ID (attribute 'seq') of the request to cancel. If missing no request is cancelled.
Expand Down
2 changes: 0 additions & 2 deletions src/dap/telemetryClassification.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,6 @@ interface IDAPOperationClassification {
'!enablecustombreakpoints.errors': { classification: 'CallstackOrException'; purpose: 'PerformanceAndHealth' };
disablecustombreakpoints: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth' };
'!disablecustombreakpoints.errors': { classification: 'CallstackOrException'; purpose: 'PerformanceAndHealth' };
canprettyprintsource: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth' };
'!canprettyprintsource.errors': { classification: 'CallstackOrException'; purpose: 'PerformanceAndHealth' };
prettyprintsource: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth' };
'!prettyprintsource.errors': { classification: 'CallstackOrException'; purpose: 'PerformanceAndHealth' };
toggleskipfilestatus: { classification: 'SystemMetaData'; purpose: 'PerformanceAndHealth' };
Expand Down
2 changes: 0 additions & 2 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import { registerCustomBreakpointsUI } from './ui/customBreakpointsUI';
import { debugNpmScript } from './ui/debugNpmScript';
import { DebugSessionTracker } from './ui/debugSessionTracker';
import { registerDebugTerminalUI } from './ui/debugTerminalUI';
import { PrettyPrintTrackerFactory } from './ui/prettyPrint';
import { attachProcess, pickProcess } from './ui/processPicker';
import { registerProfilingCommand } from './ui/profiling';
import { registerRequestCDPProxy } from './ui/requestCDPProxy';
Expand Down Expand Up @@ -87,7 +86,6 @@ export function activate(context: vscode.ExtensionContext) {
const debugSessionTracker = services.get(DebugSessionTracker);
debugSessionTracker.attach();

context.subscriptions.push(PrettyPrintTrackerFactory.register(debugSessionTracker));
registerCompanionBrowserLaunch(context);
registerCustomBreakpointsUI(context, debugSessionTracker);
registerDebugTerminalUI(
Expand Down
7 changes: 7 additions & 0 deletions src/ui/debugSessionTracker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,13 @@ export class DebugSessionTracker implements vscode.Disposable {
*/
public onSessionEnded = this._onSessionEndedEmitter.event;

/**
* Gets whether there's any active JS debug session.
*/
public get isDebugging() {
return this.sessions.size > 0;
}

/**
* Returns the session with the given ID.
*/
Expand Down
10 changes: 8 additions & 2 deletions src/ui/excludedCallersUI.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,13 @@ import { createHash } from 'crypto';
import { inject, injectable } from 'inversify';
import { basename } from 'path';
import * as vscode from 'vscode';
import { Commands, CustomViews, registerCommand } from '../common/contributionUtils';
import {
Commands,
ContextKey,
CustomViews,
registerCommand,
setContextKey,
} from '../common/contributionUtils';
import type Dap from '../dap/api';
import { IExtensionContribution } from '../ioc-extras';
import { DebugSessionTracker } from './debugSessionTracker';
Expand Down Expand Up @@ -155,7 +161,7 @@ export class ExcludedCallersUI

const hasCallers = this.allCallers.size > 0;
if (hasCallers !== this.lastHadCallers) {
vscode.commands.executeCommand('setContext', 'jsDebugHasExcludedCallers', hasCallers);
setContextKey(vscode.commands, ContextKey.HasExcludedCallers, hasCallers);
this.lastHadCallers = hasCallers;
}
}
Expand Down
23 changes: 23 additions & 0 deletions src/ui/manageContextKey.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*---------------------------------------------------------
* Copyright (C) Microsoft Corporation. All rights reserved.
*--------------------------------------------------------*/

import * as vscode from 'vscode';
import { ContextKey, IContextKeyTypes } from '../common/contributionUtils';

export class ManagedContextKey<T extends ContextKey> {
private _value: IContextKeyTypes[T] | undefined;

public set value(value: IContextKeyTypes[T] | undefined) {
if (value !== this._value) {
this._value = value;
vscode.commands.executeCommand('setContext', this.key, value);
}
}

public get value() {
return this._value;
}

constructor(private readonly key: T) {}
}
Loading

0 comments on commit d816d6a

Please sign in to comment.