Fix breakpoint context menu behaviour

Signed-off-by: thegecko <rob.moran@arm.com>
eclipse-theia · Nov 6, 2019 · fa9cf9d · fa9cf9d
1 parent 602f44d
commit fa9cf9d
Show file tree

Hide file tree

Showing 4 changed files with 79 additions and 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
@@ -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
@@ -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
@@ -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
@@ -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<void>;
-    editBreakpoint(breakpoint: DebugBreakpoint): Promise<void>;
-    async editBreakpoint(breakpoint: DebugBreakpoint | undefined = this.anyBreakpoint): Promise<void> {
-        if (breakpoint) {
-            await breakpoint.open();
-            const model = this.models.get(breakpoint.uri.toString());
+    async editBreakpoint(breakpointOrPosition?: DebugBreakpoint | monaco.Position): Promise<void> {
+        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);
             }
         }
     }