From 8b30e7b2c7c51053f07fd047dd2cf2a2ebcb522b Mon Sep 17 00:00:00 2001 From: Gayan Perera Date: Wed, 20 Dec 2023 15:21:38 +0100 Subject: [PATCH] feedback changes - change feature name to triggeredBy - add close button - add actions - add confirmation on toggle --- .../browser/breakpointEditorContribution.ts | 9 +- .../contrib/debug/browser/breakpointWidget.ts | 85 +++++++++++-------- .../contrib/debug/browser/breakpointsView.ts | 10 +-- .../debug/browser/debugEditorActions.ts | 31 +++++++ .../contrib/debug/browser/debugSession.ts | 2 +- .../debug/browser/media/breakpointWidget.css | 18 +++- .../workbench/contrib/debug/common/debug.ts | 7 +- .../contrib/debug/common/debugModel.ts | 30 ++++--- 8 files changed, 133 insertions(+), 59 deletions(-) diff --git a/src/vs/workbench/contrib/debug/browser/breakpointEditorContribution.ts b/src/vs/workbench/contrib/debug/browser/breakpointEditorContribution.ts index 2948b28ab89..ce8593f6705 100644 --- a/src/vs/workbench/contrib/debug/browser/breakpointEditorContribution.ts +++ b/src/vs/workbench/contrib/debug/browser/breakpointEditorContribution.ts @@ -284,7 +284,7 @@ export class BreakpointEditorContribution implements IBreakpointEditorContributi if (isShiftPressed) { breakpoints.forEach(bp => this.debugService.enableOrDisableBreakpoints(!enabled, bp)); - } else if (!env.isLinux && breakpoints.some(bp => !!bp.condition || !!bp.logMessage || !!bp.hitCondition)) { + } else if (!env.isLinux && breakpoints.some(bp => !!bp.condition || !!bp.logMessage || !!bp.hitCondition || !!bp.triggeredBy)) { // Show the dialog if there is a potential condition to be accidently lost. // Do not show dialog on linux due to electron issue freezing the mouse #50026 const logPoint = breakpoints.every(bp => !!bp.logMessage); @@ -454,6 +454,13 @@ export class BreakpointEditorContribution implements IBreakpointEditorContributi true, () => Promise.resolve(this.showBreakpointWidget(lineNumber, column, BreakpointWidgetContext.LOG_MESSAGE)) )); + actions.push(new Action( + 'addTriggerByBreakpoint', + nls.localize('addTriggerByBreakpoint', "Add Wait for breakpoint.."), + undefined, + true, + () => Promise.resolve(this.showBreakpointWidget(lineNumber, column, BreakpointWidgetContext.TRIGGER_POINT)) + )); } if (this.debugService.state === State.Stopped) { diff --git a/src/vs/workbench/contrib/debug/browser/breakpointWidget.ts b/src/vs/workbench/contrib/debug/browser/breakpointWidget.ts index dc0049e6485..fafb2f1c52d 100644 --- a/src/vs/workbench/contrib/debug/browser/breakpointWidget.ts +++ b/src/vs/workbench/contrib/debug/browser/breakpointWidget.ts @@ -40,6 +40,7 @@ import { PLAINTEXT_LANGUAGE_ID } from 'vs/editor/common/languages/modesRegistry' import { ILanguageFeaturesService } from 'vs/editor/common/services/languageFeatures'; import { defaultSelectBoxStyles } from 'vs/platform/theme/browser/defaultStyles'; import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; +import { Button } from 'vs/base/browser/ui/button/button'; const $ = dom.$; const IPrivateBreakpointWidgetService = createDecorator('privateBreakpointWidgetService'); @@ -87,7 +88,7 @@ export class BreakpointWidget extends ZoneWidget implements IPrivateBreakpointWi private breakpoint: IBreakpoint | undefined; private context: Context; private heightInPx: number | undefined; - private waitForBreakpointInput: IBreakpoint | undefined; + private triggeredByBreakpointInput: IBreakpoint | undefined; constructor(editor: ICodeEditor, private lineNumber: number, private column: number | undefined, context: Context | undefined, @IContextViewService private readonly contextViewService: IContextViewService, @@ -116,8 +117,8 @@ export class BreakpointWidget extends ZoneWidget implements IPrivateBreakpointWi this.context = Context.LOG_MESSAGE; } else if (this.breakpoint && !this.breakpoint.condition && this.breakpoint.hitCondition) { this.context = Context.HIT_COUNT; - } else if (this.breakpoint && this.breakpoint.waitFor) { - this.context = Context.WAIT_FOR_BREAKPOINT; + } else if (this.breakpoint && this.breakpoint.triggeredBy) { + this.context = Context.TRIGGER_POINT; } else { this.context = Context.CONDITION; } @@ -160,7 +161,7 @@ export class BreakpointWidget extends ZoneWidget implements IPrivateBreakpointWi } private rememberInput(): void { - if (this.context !== Context.WAIT_FOR_BREAKPOINT) { + if (this.context !== Context.TRIGGER_POINT) { const value = this.input.getModel().getValue(); switch (this.context) { case Context.LOG_MESSAGE: @@ -195,7 +196,7 @@ export class BreakpointWidget extends ZoneWidget implements IPrivateBreakpointWi protected _fillContainer(container: HTMLElement): void { this.setCssClass('breakpoint-widget'); - const selectBox = new SelectBox([{ text: nls.localize('expression', "Expression") }, { text: nls.localize('hitCount', "Hit Count") }, { text: nls.localize('logMessage', "Log Message") }, { text: nls.localize('waitBreakpoint', "Wait for breakpoint") }], this.context, this.contextViewService, defaultSelectBoxStyles, { ariaLabel: nls.localize('breakpointType', 'Breakpoint Type') }); + const selectBox = new SelectBox([{ text: nls.localize('expression', "Expression") }, { text: nls.localize('hitCount', "Hit Count") }, { text: nls.localize('logMessage', "Log Message") }, { text: nls.localize('triggeredBy', "Wait for breakpoint") }], this.context, this.contextViewService, defaultSelectBoxStyles, { ariaLabel: nls.localize('breakpointType', 'Breakpoint Type') }); this.selectContainer = $('.breakpoint-select-container'); selectBox.render(dom.append(container, this.selectContainer)); selectBox.onDidSelect(e => { @@ -213,36 +214,51 @@ export class BreakpointWidget extends ZoneWidget implements IPrivateBreakpointWi })); this.input.setPosition({ lineNumber: 1, column: this.input.getModel().getLineMaxColumn(1) }); - const breakpoints = this.debugService.getModel().getBreakpoints().filter(bp => bp !== this.breakpoint); - const index = breakpoints.findIndex((bp) => { return this.breakpoint?.waitFor?.matches(bp); }); - let select = 0; - if (index > -1) { - select = index + 1; - } - const items: ISelectOptionItem[] = [{ text: nls.localize('noBreakpointDependency', 'None') }]; - breakpoints.map(bp => { text: `${bp.uri.path} : ${bp.lineNumber}` }) - .forEach(i => items.push(i)); - - const selectBreakpointBox = new SelectBox(items, select, this.contextViewService, defaultSelectBoxStyles, { ariaLabel: nls.localize('selectBreakpoint', 'Select breakpoint') }); - selectBreakpointBox.onDidSelect(e => { - if (e.index === 0) { - this.waitForBreakpointInput = undefined; - } else { - this.waitForBreakpointInput = breakpoints[e.index - 1]; - } - this.close(true); - }); - this.toDispose.push(selectBreakpointBox); - this.selectBreakpointContainer = $('.select-breakpoint-container'); - selectBreakpointBox.render(dom.append(container, this.selectBreakpointContainer)); + this.createTriggerBreakpointInput(container); this.updateContextInput(); // Due to an electron bug we have to do the timeout, otherwise we do not get focus setTimeout(() => this.input.focus(), 150); } + private createTriggerBreakpointInput(container: HTMLElement) { + const breakpoints = this.debugService.getModel().getBreakpoints().filter(bp => bp !== this.breakpoint); + + const index = breakpoints.findIndex((bp) => { return this.breakpoint?.triggeredBy?.matches(bp); }); + let select = 0; + if (index > -1) { + select = index + 1; + } + const items: ISelectOptionItem[] = [{ text: nls.localize('noTriggerByBreakpoint', 'None') }]; + breakpoints.map(bp => { text: `${bp.uri.path} : ${bp.lineNumber}` }) + .forEach(i => items.push(i)); + + const selectBreakpointBox = new SelectBox(items, select, this.contextViewService, defaultSelectBoxStyles, { ariaLabel: nls.localize('selectBreakpoint', 'Select breakpoint') }); + selectBreakpointBox.onDidSelect(e => { + if (e.index === 0) { + this.triggeredByBreakpointInput = undefined; + } else { + this.triggeredByBreakpointInput = breakpoints[e.index - 1]; + } + this.close(true); + }); + this.toDispose.push(selectBreakpointBox); + this.selectBreakpointContainer = $('.select-breakpoint-container'); + + const selectionWrapper = $('.select-box-container'); + dom.append(this.selectBreakpointContainer, selectionWrapper); + selectBreakpointBox.render(selectionWrapper); + + dom.append(container, this.selectBreakpointContainer); + + const closeButton = new Button(this.selectBreakpointContainer, {}); + closeButton.label = nls.localize('close', "Close"); + closeButton.onDidClick(() => this.close(false)); + this.toDispose.push(closeButton); + } + private updateContextInput() { - if (this.context === Context.WAIT_FOR_BREAKPOINT) { + if (this.context === Context.TRIGGER_POINT) { this.inputContainer.hidden = true; this.selectBreakpointContainer.hidden = false; } else { @@ -362,8 +378,8 @@ export class BreakpointWidget extends ZoneWidget implements IPrivateBreakpointWi let condition = this.breakpoint && this.breakpoint.condition; let hitCondition = this.breakpoint && this.breakpoint.hitCondition; let logMessage = this.breakpoint && this.breakpoint.logMessage; - let waitFor = this.breakpoint && this.breakpoint.waitFor && - this.debugService.getModel().getBreakpoints().find(b => this.breakpoint!.waitFor!.matches(b)); + let triggeredBy = this.breakpoint && this.breakpoint.triggeredBy && + this.debugService.getModel().getBreakpoints().find(b => this.breakpoint!.triggeredBy!.matches(b)); this.rememberInput(); @@ -376,8 +392,8 @@ export class BreakpointWidget extends ZoneWidget implements IPrivateBreakpointWi if (this.logMessageInput || this.context === Context.LOG_MESSAGE) { logMessage = this.logMessageInput; } - if (this.context === Context.WAIT_FOR_BREAKPOINT) { - waitFor = this.waitForBreakpointInput; + if (this.context === Context.TRIGGER_POINT) { + triggeredBy = this.triggeredByBreakpointInput; } if (this.breakpoint) { @@ -386,7 +402,7 @@ export class BreakpointWidget extends ZoneWidget implements IPrivateBreakpointWi condition, hitCondition, logMessage, - waitFor + triggeredBy }); this.debugService.updateBreakpoints(this.breakpoint.originalUri, data, false).then(undefined, onUnexpectedError); } else { @@ -398,7 +414,8 @@ export class BreakpointWidget extends ZoneWidget implements IPrivateBreakpointWi enabled: true, condition, hitCondition, - logMessage + logMessage, + triggeredBy }]); } } diff --git a/src/vs/workbench/contrib/debug/browser/breakpointsView.ts b/src/vs/workbench/contrib/debug/browser/breakpointsView.ts index 1f656c3f065..5a3820b3355 100644 --- a/src/vs/workbench/contrib/debug/browser/breakpointsView.ts +++ b/src/vs/workbench/contrib/debug/browser/breakpointsView.ts @@ -1292,12 +1292,12 @@ export function getBreakpointMessageAndIcon(state: State, breakpointsActivated: } // can change this when all breakpoint supports dependent breakpoint condition - let waitForBreakpoint: BreakpointReference | undefined; + let triggeredByBreakpoint: BreakpointReference | undefined; if (breakpoint instanceof Breakpoint) { - waitForBreakpoint = breakpoint.waitFor; + triggeredByBreakpoint = breakpoint.triggeredBy; } - if (breakpoint.logMessage || breakpoint.condition || breakpoint.hitCondition || waitForBreakpoint) { + if (breakpoint.logMessage || breakpoint.condition || breakpoint.hitCondition || triggeredByBreakpoint) { const messages: string[] = []; let icon = breakpoint.logMessage ? icons.logBreakpoint.regular : icons.conditionalBreakpoint.regular; if (!breakpoint.supported) { @@ -1315,8 +1315,8 @@ export function getBreakpointMessageAndIcon(state: State, breakpointsActivated: messages.push(localize('hitCount', "Hit Count: {0}", breakpoint.hitCondition)); } - if (waitForBreakpoint) { - messages.push(localize('waitFor', "Hit after breakpoint: {0}:{1}", waitForBreakpoint.uri.toString(), waitForBreakpoint.lineNumber)); + if (triggeredByBreakpoint) { + messages.push(localize('triggeredBy', "Hit after breakpoint: {0}:{1}", triggeredByBreakpoint.uri.toString(), triggeredByBreakpoint.lineNumber)); } return { diff --git a/src/vs/workbench/contrib/debug/browser/debugEditorActions.ts b/src/vs/workbench/contrib/debug/browser/debugEditorActions.ts index 415a2df0083..aebf6c90d3d 100644 --- a/src/vs/workbench/contrib/debug/browser/debugEditorActions.ts +++ b/src/vs/workbench/contrib/debug/browser/debugEditorActions.ts @@ -151,6 +151,36 @@ class LogPointAction extends EditorAction { } } +class TriggerByBreakpointAction extends EditorAction { + + constructor() { + super({ + id: 'editor.debug.action.triggerByBreakpoint', + label: nls.localize('triggerByBreakpointEditorAction', "Debug: Wait for breakpoint..."), + precondition: CONTEXT_DEBUGGERS_AVAILABLE, + alias: 'Debug: Wait for breakpoint...', + menuOpts: [ + { + menuId: MenuId.MenubarNewBreakpointMenu, + title: nls.localize({ key: 'miTriggerByBreakpoint', comment: ['&& denotes a mnemonic'] }, "&&Wait for breakpoint..."), + group: '1_breakpoints', + order: 4, + when: CONTEXT_DEBUGGERS_AVAILABLE, + } + ] + }); + } + + async run(accessor: ServicesAccessor, editor: ICodeEditor): Promise { + const debugService = accessor.get(IDebugService); + + const position = editor.getPosition(); + if (position && editor.hasModel() && debugService.canSetBreakpointsIn(editor.getModel())) { + editor.getContribution(BREAKPOINT_EDITOR_CONTRIBUTION_ID)?.showBreakpointWidget(position.lineNumber, position.column, BreakpointWidgetContext.TRIGGER_POINT); + } + } +} + class EditBreakpointAction extends EditorAction { constructor() { super({ @@ -596,6 +626,7 @@ registerAction2(ToggleDisassemblyViewSourceCodeAction); registerAction2(ToggleBreakpointAction); registerEditorAction(ConditionalBreakpointAction); registerEditorAction(LogPointAction); +registerEditorAction(TriggerByBreakpointAction); registerEditorAction(EditBreakpointAction); registerEditorAction(RunToCursorAction); registerEditorAction(StepIntoTargetsAction); diff --git a/src/vs/workbench/contrib/debug/browser/debugSession.ts b/src/vs/workbench/contrib/debug/browser/debugSession.ts index c6cdf36a14a..20dd6e53c4b 100644 --- a/src/vs/workbench/contrib/debug/browser/debugSession.ts +++ b/src/vs/workbench/contrib/debug/browser/debugSession.ts @@ -1312,7 +1312,7 @@ export class DebugSession implements IDebugSession, IDisposable { const uriBreakpoints = new Map(); this.model.getBreakpoints({ dependentOnly: true, enabledOnly: true }).forEach(bp => { breakpoints.forEach(cbp => { - if (bp.waitFor?.matches(cbp)) { + if (bp.triggeredBy?.matches(cbp)) { const uri = bp.uri; if (!uriBreakpoints.has(uri)) { uriBreakpoints.set(uri, []); diff --git a/src/vs/workbench/contrib/debug/browser/media/breakpointWidget.css b/src/vs/workbench/contrib/debug/browser/media/breakpointWidget.css index 71f25c6641a..0af62bf27b9 100644 --- a/src/vs/workbench/contrib/debug/browser/media/breakpointWidget.css +++ b/src/vs/workbench/contrib/debug/browser/media/breakpointWidget.css @@ -31,17 +31,29 @@ } .monaco-editor .zone-widget .zone-widget-container.breakpoint-widget .select-breakpoint-container { + display: flex; + flex-direction: row; + padding: 0 10px; + flex-shrink: 1; +} + +.monaco-editor .zone-widget .zone-widget-container.breakpoint-widget .select-breakpoint-container .monaco-button { + min-width: 100px; + min-height: 18px; + padding: 2px 20px 2px 1px; +} + +.monaco-editor .zone-widget .zone-widget-container.breakpoint-widget .select-breakpoint-container .select-box-container { display: flex; justify-content: center; flex-direction: column; - padding: 0 10px; flex-shrink: 0; } -.monaco-editor .zone-widget .zone-widget-container.breakpoint-widget .select-breakpoint-container .monaco-select-box { +.monaco-editor .zone-widget .zone-widget-container.breakpoint-widget .select-breakpoint-container .select-box-container .monaco-select-box { min-width: 100px; min-height: 18px; - padding: 2px 20px 2px 8px; + padding: 2px 1px 2px 0px; } .monaco-editor .zone-widget .zone-widget-container.breakpoint-widget .select-breakpoint-container:after { diff --git a/src/vs/workbench/contrib/debug/common/debug.ts b/src/vs/workbench/contrib/debug/common/debug.ts index 9e2f53c862b..2eb4610ee0e 100644 --- a/src/vs/workbench/contrib/debug/common/debug.ts +++ b/src/vs/workbench/contrib/debug/common/debug.ts @@ -531,6 +531,7 @@ export interface IBreakpointData { readonly condition?: string; readonly logMessage?: string; readonly hitCondition?: string; + readonly triggeredBy?: IBreakpoint; } export interface IBreakpointUpdateData { @@ -539,7 +540,7 @@ export interface IBreakpointUpdateData { readonly logMessage?: string; readonly lineNumber?: number; readonly column?: number; - readonly waitFor?: IBreakpoint; + readonly triggeredBy?: IBreakpoint; } export interface IBaseBreakpoint extends IEnablement { @@ -571,7 +572,7 @@ export interface IBreakpoint extends IBaseBreakpoint { readonly endColumn?: number; readonly adapterData: any; readonly sessionAgnosticData: { lineNumber: number; column: number | undefined }; - readonly waitFor?: IBreakpointReference; + readonly triggeredBy?: IBreakpointReference; } export interface IFunctionBreakpoint extends IBaseBreakpoint { @@ -1213,7 +1214,7 @@ export const enum BreakpointWidgetContext { CONDITION = 0, HIT_COUNT = 1, LOG_MESSAGE = 2, - WAIT_FOR_BREAKPOINT = 3 + TRIGGER_POINT = 3 } export interface IDebugEditorContribution extends editorCommon.IEditorContribution { diff --git a/src/vs/workbench/contrib/debug/common/debugModel.ts b/src/vs/workbench/contrib/debug/common/debugModel.ts index 4e4206960ff..91f5b27ff89 100644 --- a/src/vs/workbench/contrib/debug/common/debugModel.ts +++ b/src/vs/workbench/contrib/debug/common/debugModel.ts @@ -882,7 +882,7 @@ export class Breakpoint extends BaseBreakpoint implements IBreakpoint { private readonly uriIdentityService: IUriIdentityService, private readonly logService: ILogService, id = generateUuid(), - public waitFor: IBreakpointReference | undefined = undefined + public triggeredBy: IBreakpointReference | undefined = undefined ) { super(enabled, hitCondition, condition, logMessage, id); } @@ -900,7 +900,7 @@ export class Breakpoint extends BaseBreakpoint implements IBreakpoint { return this.data.verified && !this.textFileService.isDirty(this._uri); } - return !this.waitFor; + return !this.triggeredBy; } get uri(): uri { @@ -969,11 +969,11 @@ export class Breakpoint extends BaseBreakpoint implements IBreakpoint { result.column = this._column; result.adapterData = this.adapterData; - if (this.waitFor) { + if (this.triggeredBy) { const wf = Object.create(null); - wf.uri = this.waitFor?.uri.toString(); - wf.lineNumber = this.waitFor?.lineNumber; - wf.column = this.waitFor?.column; + wf.uri = this.triggeredBy?.uri.toString(); + wf.lineNumber = this.triggeredBy?.lineNumber; + wf.column = this.triggeredBy?.column; result.waitFor = wf; } @@ -1000,10 +1000,10 @@ export class Breakpoint extends BaseBreakpoint implements IBreakpoint { if (!isUndefinedOrNull(data.logMessage)) { this.logMessage = data.logMessage; } - if (!isUndefinedOrNull(data.waitFor)) { - this.waitFor = new BreakpointReference(data.waitFor.uri, data.waitFor.lineNumber, data.waitFor.column); + if (!isUndefinedOrNull(data.triggeredBy)) { + this.triggeredBy = new BreakpointReference(data.triggeredBy.uri, data.triggeredBy.lineNumber, data.triggeredBy.column); } else { - this.waitFor = undefined; + this.triggeredBy = undefined; } } } @@ -1412,10 +1412,10 @@ export class DebugModel extends Disposable implements IDebugModel { if (filter.enabledOnly && (!this.breakpointsActivated || !bp.enabled)) { return false; } - if (filter.dependentOnly && bp.waitFor === undefined) { + if (filter.dependentOnly && bp.triggeredBy === undefined) { return false; } - if (filter.excludeDependent && bp.waitFor !== undefined) { + if (filter.excludeDependent && bp.triggeredBy !== undefined) { return false; } @@ -1492,7 +1492,13 @@ export class DebugModel extends Disposable implements IDebugModel { } addBreakpoints(uri: uri, rawData: IBreakpointData[], fireEvent = true): IBreakpoint[] { - const newBreakpoints = rawData.map(rawBp => new Breakpoint(uri, rawBp.lineNumber, rawBp.column, rawBp.enabled === false ? false : true, rawBp.condition, rawBp.hitCondition, rawBp.logMessage, undefined, this.textFileService, this.uriIdentityService, this.logService, rawBp.id)); + const newBreakpoints = rawData.map(rawBp => { + let triggeredBy = undefined; + if (rawBp!.triggeredBy) { + triggeredBy = new BreakpointReference(rawBp.triggeredBy.uri, rawBp.triggeredBy.lineNumber, rawBp.triggeredBy.column); + } + return new Breakpoint(uri, rawBp.lineNumber, rawBp.column, rawBp.enabled === false ? false : true, rawBp.condition, rawBp.hitCondition, rawBp.logMessage, undefined, this.textFileService, this.uriIdentityService, this.logService, rawBp.id, triggeredBy); + }); this.breakpoints = this.breakpoints.concat(newBreakpoints); this.breakpointsActivated = true; this.sortAndDeDup();