Implement Go To Next/Previous Breakpoint editor actions #50285
Conversation
weinand
added
feature-request
|
|
||
| public run(accessor: ServicesAccessor, editor: ICodeEditor, args: any): void | TPromise<void, any> { | ||
| const debugService = accessor.get(IDebugService); | ||
| const editorService = accessor.get(IEditorService); |
There was a problem hiding this comment.
It's not really checked in any other place in the file ;-)
| const currentLine = editor.getPosition().lineNumber; | ||
| const allEnabledBreakpoints = | ||
| debugService.getModel().getBreakpoints() | ||
| .filter(bp => bp.enabled) |
There was a problem hiding this comment.
Instead of doing a .filter on top, please pass a "enabledOnly: true" to the getBreakpoints
| } | ||
|
|
||
| if (moveBreakpoint) { | ||
| const selection = new Selection(moveBreakpoint.lineNumber, moveBreakpoint.column || 0, moveBreakpoint.lineNumber, moveBreakpoint.column || 0); |
There was a problem hiding this comment.
Use openBreakpointSource instead
| debugService.getModel().getBreakpoints() | ||
| .filter(bp => bp.enabled) | ||
| .sort((a, b) => { | ||
| if (this.isNext) { |
There was a problem hiding this comment.
Can we do this in twice as less lines? seems like a lot of code is duplicated in the next 20 lines?
There was a problem hiding this comment.
The sorting is unnecessary in the end since debugModel is responsible for sorting its own breakpoints - https://github.com/Microsoft/vscode/blob/cdd8256102433bf52c2f5b05aea312775a34a669/src/vs/workbench/parts/debug/common/debugModel.ts#L1003-L1015
|
@Krzysztof-Cieslak thanks a lot for your PR. I have commented directly in the file. |
|
You seem to have rebased instead of merge. I suggest to create a new branch to cleanup the mess. |
|
I've figured out my small problems with git, I think it's more or less ready ;-) |
|
@Krzysztof-Cieslak thanks a lot, this looks great. Merged it in 🍻 |
Fix #31806
CC: @isidorn