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
Fixes #46509 - Added key binding to disable a single breakpoint #46629
Conversation
Thanks for your PR, however currently we are wrapping up the milestone so I will only have time to review this next milestone -> April |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine to me!
@marcobeltempo thanks for your pr. I left feedback directly in the code, any way not something we will take into for april, pushing out. |
@@ -44,6 +44,29 @@ export function registerCommands(): void { | |||
} | |||
}); | |||
|
|||
KeybindingsRegistry.registerCommandAndKeybindingRule({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would expect this command to be toggle enablement. So if my breakpoint is enabled to disable it and the other way around.
That way it is more flexible. In order to change that the name and the id should be changed.
KeybindingsRegistry.registerCommandAndKeybindingRule({ | ||
id: 'debug.disableBreakpoint', | ||
weight: KeybindingsRegistry.WEIGHT.workbenchContrib(), | ||
primary: KeyMod.CtrlCmd | KeyMod.Alt | KeyCode.KEY_D, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No default keybiinding please.
src/vs/code/electron-main/menus.ts
Outdated
@@ -850,6 +850,7 @@ export class CodeMenu { | |||
const continueAction = this.createMenuItem(nls.localize({ key: 'miContinue', comment: ['&& denotes a mnemonic'] }, "&&Continue"), 'workbench.action.debug.continue'); | |||
|
|||
const toggleBreakpoint = this.createMenuItem(nls.localize({ key: 'miToggleBreakpoint', comment: ['&& denotes a mnemonic'] }, "Toggle &&Breakpoint"), 'editor.debug.action.toggleBreakpoint'); | |||
const disableBreakpoint = this.createMenuItem(nls.localize({ key: 'miDisableBreakpoint', comment: ['&& denotes a mnemonic'] }, "Disable &&Breakpoint"), 'debug.disableBreakpoint'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would not want this command in the debug menu, since it only works if there is a breakpoint on a line, and the debug menu is too global. So I would remove all this.
@isidorn thank you for the feedback. I've made the requested changes. |
Thanks. This looks good, however this week we are in endgame week so we are focusing on testing. |
@marcobeltempo thank you for your PR, merging it in 🍻 |
Fixes #46509
This PR includes support to enable/disable a single breakpoint
using the quick open menu or thekeyboard shortcut.Ctrl+Alt+D
NOTE