-
Notifications
You must be signed in to change notification settings - Fork 300
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
put BP on first code line #7324
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,8 @@ import { Commands, Identifiers } from '../../datascience/constants'; | |
import { IKernel } from '../../datascience/jupyter/kernels/types'; | ||
import { sendTelemetryEvent } from '../../telemetry'; | ||
import { DebuggingTelemetry } from '../constants'; | ||
import { parseForComments } from '../../../datascience-ui/common'; | ||
import { noop } from '../../common/utils/misc'; | ||
|
||
interface dumpCellResponse { | ||
sourcePath: string; // filename for the dumped source | ||
|
@@ -496,31 +498,47 @@ export class KernelDebugAdapter implements DebugAdapter, IKernelDebugAdapter, ID | |
await this.dumpCell(cell.document.uri.toString()); | ||
|
||
if (this.configuration.__mode === KernelDebugMode.RunByLine) { | ||
const initialBreakpoint: DebugProtocol.SourceBreakpoint = { | ||
line: 1 | ||
}; | ||
const splitPath = cell.notebook.uri.path.split('/'); | ||
const name = splitPath[splitPath.length - 1]; | ||
const message: DebugProtocol.SetBreakpointsRequest = { | ||
seq: seq + 1, | ||
type: 'request', | ||
command: 'setBreakpoints', | ||
arguments: { | ||
source: { | ||
name: name, | ||
path: cell.document.uri.toString() | ||
}, | ||
lines: [1], | ||
breakpoints: [initialBreakpoint], | ||
sourceModified: false | ||
// This will save the code lines of the cell in lineList (so ignore comments and emtpy lines) | ||
// Its done to set the Run by Line breakpoint on the first code line | ||
const textLines = cell.document.getText().splitLines({ trim: false, removeEmptyEntries: false }); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you add a comment here explaining why we need to parseForComments? I assume this is because before the breakpoint would end up on a commented line and not work. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that's it, will do |
||
const lineList: number[] = []; | ||
parseForComments( | ||
textLines, | ||
() => noop(), | ||
(s, i) => { | ||
if (s.trim().length !== 0) { | ||
lineList.push(i); | ||
} | ||
} | ||
}; | ||
this.sendRequestToJupyterSession(message); | ||
); | ||
lineList.sort(); | ||
|
||
// Don't send the SetBreakpointsRequest or open the variable view if there are no code lines | ||
if (lineList.length !== 0) { | ||
const initialBreakpoint: DebugProtocol.SourceBreakpoint = { | ||
line: lineList[0] + 1 | ||
}; | ||
const message: DebugProtocol.SetBreakpointsRequest = { | ||
seq: seq + 1, | ||
type: 'request', | ||
command: 'setBreakpoints', | ||
arguments: { | ||
source: { | ||
name: path.basename(cell.notebook.uri.path), | ||
path: cell.document.uri.toString() | ||
}, | ||
lines: [lineList[0] + 1], | ||
breakpoints: [initialBreakpoint], | ||
sourceModified: false | ||
} | ||
}; | ||
this.sendRequestToJupyterSession(message); | ||
|
||
// Open variable view | ||
const settings = this.settings.getSettings(); | ||
if (settings.showVariableViewWhenDebugging) { | ||
await this.commandManager.executeCommand(Commands.OpenVariableView); | ||
// Open variable view | ||
const settings = this.settings.getSettings(); | ||
if (settings.showVariableViewWhenDebugging) { | ||
void this.commandManager.executeCommand(Commands.OpenVariableView); | ||
} | ||
} | ||
} | ||
|
||
|
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.
We should re-open #7251 or create a new issue to track the work around, else I do'nt see how we'll end up removing the work around.
I.e. theres no issue to track this work.
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'll create a new one
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.
#7339