From 23b1c07dd38ab1854a6e5b620c8aaeed14a111c0 Mon Sep 17 00:00:00 2001 From: thegecko Date: Fri, 1 Nov 2019 14:28:59 +0000 Subject: [PATCH] Fix breakpoint context menu behaviour Signed-off-by: thegecko --- .../menu/electron-context-menu-renderer.ts | 2 +- ...debug-frontend-application-contribution.ts | 70 ++++++++++--------- .../src/browser/editor/debug-editor-model.ts | 27 ++++--- .../browser/editor/debug-editor-service.ts | 55 ++++++++------- 4 files changed, 79 insertions(+), 75 deletions(-) diff --git a/packages/core/src/electron-browser/menu/electron-context-menu-renderer.ts b/packages/core/src/electron-browser/menu/electron-context-menu-renderer.ts index 56769d0985c0f..5e15a4a95f9e8 100644 --- a/packages/core/src/electron-browser/menu/electron-context-menu-renderer.ts +++ b/packages/core/src/electron-browser/menu/electron-context-menu-renderer.ts @@ -38,7 +38,7 @@ export class ElectronContextMenuRenderer implements ContextMenuRenderer { // native context menu stops the event loop, so there is no keyboard events this.context.resetAltPressed(); if (onHide) { - onHide(); + menu.once('menu-will-close', () => onHide()); } } diff --git a/packages/debug/src/browser/debug-frontend-application-contribution.ts b/packages/debug/src/browser/debug-frontend-application-contribution.ts index b84e8f9180163..1fff3913d3753 100644 --- a/packages/debug/src/browser/debug-frontend-application-contribution.ts +++ b/packages/debug/src/browser/debug-frontend-application-contribution.ts @@ -747,60 +747,61 @@ export class DebugFrontendApplicationContribution extends AbstractViewContributi isVisible: () => !!this.selectedVariable && this.selectedVariable.supportCopyAsExpression }); + // Debug context menu commands registry.registerCommand(DebugEditorContextCommands.ADD_BREAKPOINT, { - execute: () => this.editors.toggleBreakpoint(), - isEnabled: () => !this.editors.anyBreakpoint, - isVisible: () => !this.editors.anyBreakpoint + execute: position => this.isPosition(position) && this.editors.toggleBreakpoint(position), + isEnabled: position => this.isPosition(position) && !this.editors.anyBreakpoint(position), + isVisible: position => this.isPosition(position) && !this.editors.anyBreakpoint(position) }); registry.registerCommand(DebugEditorContextCommands.ADD_CONDITIONAL_BREAKPOINT, { - execute: () => this.editors.addBreakpoint('condition'), - isEnabled: () => !this.editors.anyBreakpoint, - isVisible: () => !this.editors.anyBreakpoint + execute: position => this.isPosition(position) && this.editors.addBreakpoint('condition', position), + isEnabled: position => this.isPosition(position) && !this.editors.anyBreakpoint(position), + isVisible: position => this.isPosition(position) && !this.editors.anyBreakpoint(position) }); registry.registerCommand(DebugEditorContextCommands.ADD_LOGPOINT, { - execute: () => this.editors.addBreakpoint('logMessage'), - isEnabled: () => !this.editors.anyBreakpoint, - isVisible: () => !this.editors.anyBreakpoint + execute: position => this.isPosition(position) && this.editors.addBreakpoint('logMessage', position), + isEnabled: position => this.isPosition(position) && !this.editors.anyBreakpoint(position), + isVisible: position => this.isPosition(position) && !this.editors.anyBreakpoint(position) }); registry.registerCommand(DebugEditorContextCommands.REMOVE_BREAKPOINT, { - execute: () => this.editors.toggleBreakpoint(), - isEnabled: () => !!this.editors.breakpoint, - isVisible: () => !!this.editors.breakpoint + execute: position => this.isPosition(position) && this.editors.toggleBreakpoint(position), + isEnabled: position => this.isPosition(position) && !!this.editors.getBreakpoint(position), + isVisible: position => this.isPosition(position) && !!this.editors.getBreakpoint(position) }); registry.registerCommand(DebugEditorContextCommands.EDIT_BREAKPOINT, { - execute: () => this.editors.editBreakpoint(), - isEnabled: () => !!this.editors.breakpoint, - isVisible: () => !!this.editors.breakpoint + execute: position => this.isPosition(position) && this.editors.editBreakpoint(position), + isEnabled: position => this.isPosition(position) && !!this.editors.getBreakpoint(position), + isVisible: position => this.isPosition(position) && !!this.editors.getBreakpoint(position) }); registry.registerCommand(DebugEditorContextCommands.ENABLE_BREAKPOINT, { - execute: () => this.editors.setBreakpointEnabled(true), - isEnabled: () => this.editors.breakpointEnabled === false, - isVisible: () => this.editors.breakpointEnabled === false + execute: position => this.isPosition(position) && this.editors.setBreakpointEnabled(position, true), + isEnabled: position => this.isPosition(position) && this.editors.getBreakpointEnabled(position) === false, + isVisible: position => this.isPosition(position) && this.editors.getBreakpointEnabled(position) === false }); registry.registerCommand(DebugEditorContextCommands.DISABLE_BREAKPOINT, { - execute: () => this.editors.setBreakpointEnabled(false), - isEnabled: () => !!this.editors.breakpointEnabled, - isVisible: () => !!this.editors.breakpointEnabled + execute: position => this.isPosition(position) && this.editors.setBreakpointEnabled(position, false), + isEnabled: position => this.isPosition(position) && !!this.editors.getBreakpointEnabled(position), + isVisible: position => this.isPosition(position) && !!this.editors.getBreakpointEnabled(position) }); registry.registerCommand(DebugEditorContextCommands.REMOVE_LOGPOINT, { - execute: () => this.editors.toggleBreakpoint(), - isEnabled: () => !!this.editors.logpoint, - isVisible: () => !!this.editors.logpoint + execute: position => this.isPosition(position) && this.editors.toggleBreakpoint(position), + isEnabled: position => this.isPosition(position) && !!this.editors.getLogpoint(position), + isVisible: position => this.isPosition(position) && !!this.editors.getLogpoint(position) }); registry.registerCommand(DebugEditorContextCommands.EDIT_LOGPOINT, { - execute: () => this.editors.editBreakpoint(), - isEnabled: () => !!this.editors.logpoint, - isVisible: () => !!this.editors.logpoint + execute: position => this.isPosition(position) && this.editors.editBreakpoint(position), + isEnabled: position => this.isPosition(position) && !!this.editors.getLogpoint(position), + isVisible: position => this.isPosition(position) && !!this.editors.getLogpoint(position) }); registry.registerCommand(DebugEditorContextCommands.ENABLE_LOGPOINT, { - execute: () => this.editors.setBreakpointEnabled(true), - isEnabled: () => this.editors.logpointEnabled === false, - isVisible: () => this.editors.logpointEnabled === false + execute: position => this.isPosition(position) && this.editors.setBreakpointEnabled(position, true), + isEnabled: position => this.isPosition(position) && this.editors.getLogpointEnabled(position) === false, + isVisible: position => this.isPosition(position) && this.editors.getLogpointEnabled(position) === false }); registry.registerCommand(DebugEditorContextCommands.DISABLE_LOGPOINT, { - execute: () => this.editors.setBreakpointEnabled(false), - isEnabled: () => !!this.editors.logpointEnabled, - isVisible: () => !!this.editors.logpointEnabled + execute: position => this.isPosition(position) && this.editors.setBreakpointEnabled(position, false), + isEnabled: position => this.isPosition(position) && !!this.editors.getLogpointEnabled(position), + isVisible: position => this.isPosition(position) && !!this.editors.getLogpointEnabled(position) }); registry.registerCommand(DebugBreakpointWidgetCommands.ACCEPT, { @@ -1009,4 +1010,7 @@ export class DebugFrontendApplicationContribution extends AbstractViewContributi return variables && variables.selectedElement instanceof DebugVariable && variables.selectedElement || undefined; } + protected isPosition(position: monaco.Position): boolean { + return (position instanceof monaco.Position); + } } diff --git a/packages/debug/src/browser/editor/debug-editor-model.ts b/packages/debug/src/browser/editor/debug-editor-model.ts index 090b2cbcd49e5..022090b5d64fe 100644 --- a/packages/debug/src/browser/editor/debug-editor-model.ts +++ b/packages/debug/src/browser/editor/debug-editor-model.ts @@ -240,20 +240,13 @@ export class DebugEditorModel implements Disposable { return breakpoints; } - protected _position: monaco.Position | undefined; get position(): monaco.Position { - return this._position || this.editor.getControl().getPosition()!; + return this.editor.getControl().getPosition()!; } - get breakpoint(): DebugBreakpoint | undefined { - return this.getBreakpoint(); - } - protected getBreakpoint(position: monaco.Position = this.position): DebugBreakpoint | undefined { + getBreakpoint(position: monaco.Position = this.position): DebugBreakpoint | undefined { return this.sessions.getBreakpoint(this.uri, position.lineNumber); } - toggleBreakpoint(): void { - this.doToggleBreakpoint(); - } - protected doToggleBreakpoint(position: monaco.Position = this.position): void { + toggleBreakpoint(position: monaco.Position = this.position): void { const breakpoint = this.getBreakpoint(position); if (breakpoint) { breakpoint.remove(); @@ -285,12 +278,16 @@ export class DebugEditorModel implements Disposable { protected handleMouseDown(event: monaco.editor.IEditorMouseEvent): void { if (event.target && event.target.type === monaco.editor.MouseTargetType.GUTTER_GLYPH_MARGIN) { if (event.event.rightButton) { - this._position = event.target.position!; - this.contextMenu.render(DebugEditorModel.CONTEXT_MENU, event.event.browserEvent, () => - setTimeout(() => this._position = undefined) - ); + this.editor.focus(); + setTimeout(() => { + this.contextMenu.render({ + menuPath: DebugEditorModel.CONTEXT_MENU, + anchor: event.event.browserEvent, + args: [event.target.position!] + }); + }); } else { - this.doToggleBreakpoint(event.target.position!); + this.toggleBreakpoint(event.target.position!); } } this.hintBreakpoint(event); diff --git a/packages/debug/src/browser/editor/debug-editor-service.ts b/packages/debug/src/browser/editor/debug-editor-service.ts index 9b4ea52ad4908..01fc451cc11ea 100644 --- a/packages/debug/src/browser/editor/debug-editor-service.ts +++ b/packages/debug/src/browser/editor/debug-editor-service.ts @@ -84,38 +84,38 @@ export class DebugEditorService { return uri && this.models.get(uri.toString()); } - get logpoint(): DebugBreakpoint | undefined { - const logpoint = this.anyBreakpoint; + getLogpoint(position: monaco.Position): DebugBreakpoint | undefined { + const logpoint = this.anyBreakpoint(position); return logpoint && logpoint.logMessage ? logpoint : undefined; } - get logpointEnabled(): boolean | undefined { - const { logpoint } = this; + getLogpointEnabled(position: monaco.Position): boolean | undefined { + const logpoint = this.getLogpoint(position); return logpoint && logpoint.enabled; } - get breakpoint(): DebugBreakpoint | undefined { - const breakpoint = this.anyBreakpoint; + getBreakpoint(position: monaco.Position): DebugBreakpoint | undefined { + const breakpoint = this.anyBreakpoint(position); return breakpoint && breakpoint.logMessage ? undefined : breakpoint; } - get breakpointEnabled(): boolean | undefined { - const { breakpoint } = this; + getBreakpointEnabled(position: monaco.Position): boolean | undefined { + const breakpoint = this.getBreakpoint(position); return breakpoint && breakpoint.enabled; } - get anyBreakpoint(): DebugBreakpoint | undefined { - return this.model && this.model.breakpoint; + anyBreakpoint(position?: monaco.Position): DebugBreakpoint | undefined { + return this.model && this.model.getBreakpoint(position); } - toggleBreakpoint(): void { + toggleBreakpoint(position?: monaco.Position): void { const { model } = this; if (model) { - model.toggleBreakpoint(); + model.toggleBreakpoint(position); } } - setBreakpointEnabled(enabled: boolean): void { - const { anyBreakpoint } = this; - if (anyBreakpoint) { - anyBreakpoint.setEnabled(enabled); + setBreakpointEnabled(position: monaco.Position, enabled: boolean): void { + const breakpoint = this.anyBreakpoint(position); + if (breakpoint) { + breakpoint.setEnabled(enabled); } } @@ -135,28 +135,31 @@ export class DebugEditorService { return false; } - addBreakpoint(context: DebugBreakpointWidget.Context): void { + addBreakpoint(context: DebugBreakpointWidget.Context, position?: monaco.Position): void { const { model } = this; if (model) { - const { breakpoint } = model; + position = position || model.position; + const breakpoint = model.getBreakpoint(position); if (breakpoint) { model.breakpointWidget.show({ breakpoint, context }); } else { model.breakpointWidget.show({ - position: model.position, + position, context }); } } } - editBreakpoint(): Promise; - editBreakpoint(breakpoint: DebugBreakpoint): Promise; - async editBreakpoint(breakpoint: DebugBreakpoint | undefined = this.anyBreakpoint): Promise { - if (breakpoint) { - await breakpoint.open(); - const model = this.models.get(breakpoint.uri.toString()); + async editBreakpoint(breakpointOrPosition?: DebugBreakpoint | monaco.Position): Promise { + if (breakpointOrPosition instanceof monaco.Position) { + breakpointOrPosition = this.anyBreakpoint(breakpointOrPosition); + } + + if (breakpointOrPosition) { + await breakpointOrPosition.open(); + const model = this.models.get(breakpointOrPosition.uri.toString()); if (model) { - model.breakpointWidget.show(breakpoint); + model.breakpointWidget.show(breakpointOrPosition); } } }