Conversation
|
|
| if (debugService.getConfigurationManager().canSetBreakpointsIn(editor.getModel())) { | ||
| return debugService.addBreakpoints(modelUri, [{ lineNumber: position.lineNumber }], 'debugEditorActions.toggleBreakpointAction'); | ||
| } | ||
| const lineNumbers = new Set(editor._getCursors().getAll() |
There was a problem hiding this comment.
Please use editor.getSelections() instead of editor._getCursors.
That is the proper API (I just double checked)
There was a problem hiding this comment.
When I did this I thought the Selection object was for highlighted text only, realized later on that was not the case. I made this change. In using the Selection object I am only considering the line the cursor is on. I could have done all lines that are selected, since the original did not do this, I did not either.
| .filter(cs => cs.modelState.position) | ||
| .map(cs => cs.modelState.position.lineNumber)); | ||
|
|
||
| const promises = Array<Promise<any>>(); |
There was a problem hiding this comment.
There is no need for this promises array.
You can just say return Promise.all(lineNumbers.map(..
There was a problem hiding this comment.
Made this change. Only reason I was doing that before is because there is the possibly to remove multiple breakpoints from a line, resulting in multiple promises for one iteration. I just wrapped that with a Promise.all so they resolve correctly.
|
Thanks for your PR. It looks good and it works fine. |
|
Looks great, thanks a lot for your contribution! Merging in 👏 |
3 participants
#77879