Skip to content
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.

Sign up for GitHub

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

Run by Line #6655

Merged
merged 14 commits into from
Jul 16, 2021
Merged

Run by Line #6655

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
b7845a4
add command and icon
Jul 13, 2021
cc62355
oops
Jul 13, 2021
de9759a
start in run by line mode
Jul 15, 2021
45ba955
add continue and stop command
Jul 15, 2021
1579dc3
lint
Jul 16, 2021
b280d05
lint
Jul 16, 2021
2ff747a
Merge branch 'main' into david/runByLineIcon
Jul 16, 2021
45d6f0d
cleaning
Jul 16, 2021
5941c3e
lint
Jul 16, 2021
c8ad57d
get actual threadId for RBL stepIn
Jul 16, 2021
09695da
oops
Jul 16, 2021
3ba0fe5
restore engine and use real seq number on RBL
Jul 16, 2021
405c2b7
dont use path.basename on my custom debugger name
Jul 16, 2021
4c50546
lint
Jul 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .eslintrc.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -719,7 +719,6 @@ module.exports = {
'src/client/debugger/extension/attachQuickPick/provider.ts',
'src/client/debugger/extension/attachQuickPick/picker.ts',
'src/client/debugger/extension/helpers/protocolParser.ts',
'src/client/debugger/types.ts',
'src/client/debugger/constants.ts',
'src/client/languageServices/jediProxyFactory.ts',
'src/client/languageServices/proposeLanguageServerBanner.ts',
Expand Down
62 changes: 60 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
"theme": "light"
},
"engines": {
"vscode": "1.59.0-insider"
"vscode": "^1.58.0"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is a merge issue?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, sorry

},
"keywords": [
"jupyter",
Expand Down Expand Up @@ -281,7 +281,28 @@
"title": "Debug",
"icon": "$(bug)",
"category": "Jupyter",
"enablement": "notebookKernelCount > 0 && !jupyter.notebookeditor.debuggingInProgress && config.jupyter.experimental.debugging"
"enablement": "notebookKernelCount > 0 && !jupyter.notebookeditor.debuggingInProgress && !jupyter.notebookeditor.runByLineInProgress && config.jupyter.experimental.debugging"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could the 'debuggingInProgress' context key be true when run by line is in progress? So that the enablement clause here can be shorter?

Copy link
Author

@DavidKutu DavidKutu Jul 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can't, but we don't want users to be able to press RBL while debugging and vice versa.
Also we don't want them to be able to press RBL if they're already RBLing.

},
{
"command": "jupyter.runByLine",
"title": "Run by Line",
"icon": "$(debug-line-by-line)",
"category": "Jupyter",
"enablement": "notebookKernelCount > 0 && !jupyter.notebookeditor.debuggingInProgress && !jupyter.notebookeditor.runByLineInProgress && config.jupyter.experimental.debugging"
},
{
"command": "jupyter.runByLineContinue",
"title": "Continue",
"icon": "$(debug-line-by-line)",
"category": "Jupyter",
"enablement": "notebookKernelCount > 0 && jupyter.notebookeditor.runByLineInProgress && config.jupyter.experimental.debugging"
},
{
"command": "jupyter.runByLineStop",
"title": "Stop",
"icon": "$(debug-stop)",
"category": "Jupyter",
"enablement": "notebookKernelCount > 0 && jupyter.notebookeditor.runByLineInProgress && config.jupyter.experimental.debugging"
},
{
"command": "jupyter.viewOutput",
Expand Down Expand Up @@ -930,6 +951,23 @@
"when": "notebookType == 'jupyter-notebook' && isWorkspaceTrusted && jupyter.notebookeditor.canDebug && config.jupyter.experimental.debugging"
}
],
"notebook/cell/title": [
{
"command": "jupyter.runByLine",
"when": "notebookType == jupyter-notebook && notebookCellType == code && isWorkspaceTrusted && jupyter.notebookeditor.canDebug && !jupyter.notebookeditor.runByLineInProgress && config.jupyter.experimental.debugging",
"group": "inline/cell@0"
},
{
"command": "jupyter.runByLineContinue",
"when": "notebookType == jupyter-notebook && notebookCellType == code && isWorkspaceTrusted && jupyter.notebookeditor.canDebug && jupyter.notebookeditor.runByLineInProgress && config.jupyter.experimental.debugging",
"group": "inline/cell@0"
},
{
"command": "jupyter.runByLineStop",
"when": "notebookType == jupyter-notebook && notebookCellType == code && isWorkspaceTrusted && jupyter.notebookeditor.canDebug && jupyter.notebookeditor.runByLineInProgress && config.jupyter.experimental.debugging",
"group": "inline/cell@0"
}
],
"interactive/toolbar": [
{
"command": "jupyter.interactive.clearAllCells",
Expand Down Expand Up @@ -1012,6 +1050,26 @@
}
],
"commandPalette": [
{
"command": "jupyter.debugNotebook",
"title": "Debug",
"when": "false"
},
{
"command": "jupyter.runByLine",
"title": "Run by Line",
"when": "false"
},
{
"command": "jupyter.runByLineContinue",
"title": "Continue",
"when": "false"
},
{
"command": "jupyter.runByLineStop",
"title": "Stop",
"when": "false"
},
{
"command": "jupyter.notebookeditor.keybind.toggleOutput",
"title": "%DataScience.toggleCellOutput%",
Expand Down
3 changes: 3 additions & 0 deletions src/client/common/application/commands.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,4 +167,7 @@ export interface ICommandNameArgumentTypeMapping extends ICommandNameWithoutArgu
[DSCommands.NotebookEditorKeybindSave]: [];
[DSCommands.NotebookEditorKeybindUndo]: [];
[DSCommands.DebugNotebook]: [];
[DSCommands.RunByLine]: [NotebookCell];
[DSCommands.RunByLineContinue]: [NotebookCell];
[DSCommands.RunByLineStop]: [NotebookCell];
}
4 changes: 4 additions & 0 deletions src/client/datascience/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,9 @@ export namespace Commands {
export const InteractiveExportAsNotebook = 'jupyter.interactive.exportasnotebook';
export const InteractiveExportAs = 'jupyter.interactive.exportas';
export const DebugNotebook = 'jupyter.debugNotebook';
export const RunByLine = 'jupyter.runByLine';
export const RunByLineContinue = 'jupyter.runByLineContinue';
export const RunByLineStop = 'jupyter.runByLineStop';
}

export namespace CodeLensCommands {
Expand Down Expand Up @@ -204,6 +207,7 @@ export namespace EditorContexts {
export const CanInterruptNotebookKernel = 'jupyter.notebookeditor.canInterruptNotebookKernel';
export const CanDebug = 'jupyter.notebookeditor.canDebug';
export const DebuggingInProgress = 'jupyter.notebookeditor.debuggingInProgress';
export const RunByLineInProgress = 'jupyter.notebookeditor.runByLineInProgress';
export const IsPythonNotebook = 'jupyter.ispythonnotebook';
export const IsVSCodeNotebookActive = 'jupyter.isvscodenotebookactive';
export const IsDataViewerActive = 'jupyter.dataViewerActive';
Expand Down
81 changes: 65 additions & 16 deletions src/client/debugger/jupyter/debuggingManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,14 @@
'use strict';

import { inject, injectable } from 'inversify';
import { debug, NotebookDocument, workspace, DebugAdapterInlineImplementation, DebugSession } from 'vscode';
import {
debug,
NotebookDocument,
workspace,
DebugAdapterInlineImplementation,
DebugSession,
NotebookCell
} from 'vscode';
import * as path from 'path';
import { IKernelProvider } from '../../datascience/jupyter/kernels/types';
import { IDisposable } from '../../common/types';
Expand All @@ -27,15 +34,16 @@ class Debugger {

readonly session: Promise<DebugSession>;

constructor(public readonly document: NotebookDocument) {
constructor(public readonly document: NotebookDocument, public readonly cell?: NotebookCell) {
const name = cell ? `${document.uri.toString()}?RBL=${cell.index}` : document.uri.toString();
this.session = new Promise<DebugSession>((resolve, reject) => {
this.resolveFunc = resolve;
this.rejectFunc = reject;

debug
.startDebugging(undefined, {
type: DataScience.pythonKernelDebugAdapter(),
name: `${path.basename(document.uri.toString())}`,
name: `${path.basename(name)}`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

path.basename works with ? in it? I mean I'd think that would work but seems like an invalid file name?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it does work, I use it to differentiate between run by line and normal debugging.

I'm open to change it if you have a suggestion

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nvm, I'll use it the same, but will use path.basename only on the doc URI

request: 'attach',
internalConsoleOptions: 'neverOpen',
__document: document.uri.toString()
Expand Down Expand Up @@ -67,7 +75,9 @@ class Debugger {
@injectable()
export class DebuggingManager implements IExtensionSingleActivationService, IDisposable {
private debuggingInProgress: ContextKey;
private runByLineInProgress: ContextKey;
private notebookToDebugger = new Map<NotebookDocument, Debugger>();
private notebookToDebugAdapter = new Map<NotebookDocument, KernelDebugAdapter>();
private readonly disposables: IDisposable[] = [];

public constructor(
Expand All @@ -81,14 +91,17 @@ export class DebuggingManager implements IExtensionSingleActivationService, IDis
@inject(IFileSystem) private fs: IFileSystem
) {
this.debuggingInProgress = new ContextKey(EditorContexts.DebuggingInProgress, this.commandManager);
this.runByLineInProgress = new ContextKey(EditorContexts.RunByLineInProgress, this.commandManager);
this.updateToolbar(false);
this.updateCellToolbar(false);
}

public async activate() {
this.disposables.push(
// track termination of debug sessions
debug.onDidTerminateDebugSession(async (session) => {
this.updateToolbar(false);
this.updateCellToolbar(false);
for (const [doc, dbg] of this.notebookToDebugger.entries()) {
if (dbg && session === (await dbg.session)) {
this.debuggingCellMap.getCellsAnClearQueue(doc);
Expand All @@ -104,6 +117,7 @@ export class DebuggingManager implements IExtensionSingleActivationService, IDis
const dbg = this.notebookToDebugger.get(document);
if (dbg) {
this.updateToolbar(false);
this.updateCellToolbar(false);
await dbg.stop();
}
}),
Expand All @@ -125,16 +139,16 @@ export class DebuggingManager implements IExtensionSingleActivationService, IDis
});
if (notebook && notebook.session) {
debug.resolve(session);
return new DebugAdapterInlineImplementation(
new KernelDebugAdapter(
session,
debug.document,
notebook.session,
this.debuggingCellMap,
this.commandManager,
this.fs
)
const adapter = new KernelDebugAdapter(
session,
debug.document,
notebook.session,
this.debuggingCellMap,
this.commandManager,
this.fs
);
this.notebookToDebugAdapter.set(debug.document, adapter);
return new DebugAdapterInlineImplementation(adapter);
} else {
void this.appShell.showInformationMessage(DataScience.kernelWasNotStarted());
}
Expand All @@ -153,6 +167,35 @@ export class DebuggingManager implements IExtensionSingleActivationService, IDis
} else {
void this.appShell.showErrorMessage(DataScience.noNotebookToDebug());
}
}),

this.commandManager.registerCommand(DSCommands.RunByLine, (cell: NotebookCell) => {
const editor = this.vscNotebook.activeNotebookEditor;
if (editor) {
this.updateToolbar(true);
this.updateCellToolbar(true);
void this.startDebugging(editor.document, cell);
} else {
void this.appShell.showErrorMessage(DataScience.noNotebookToDebug());
}
}),

this.commandManager.registerCommand(DSCommands.RunByLineContinue, (cell: NotebookCell) => {
const adapter = this.notebookToDebugAdapter.get(cell.notebook);
if (adapter) {
adapter.runByLineContinue();
} else {
void this.appShell.showErrorMessage(DataScience.noNotebookToDebug());
}
}),

this.commandManager.registerCommand(DSCommands.RunByLineStop, (cell: NotebookCell) => {
const adapter = this.notebookToDebugAdapter.get(cell.notebook);
if (adapter) {
adapter.runByLineStop();
} else {
void this.appShell.showErrorMessage(DataScience.noNotebookToDebug());
}
})
);
}
Expand All @@ -165,17 +208,23 @@ export class DebuggingManager implements IExtensionSingleActivationService, IDis
this.debuggingInProgress.set(debugging).ignoreErrors();
}

private async startDebugging(doc: NotebookDocument) {
private updateCellToolbar(runningByLine: boolean) {
this.runByLineInProgress.set(runningByLine).ignoreErrors();
}

private async startDebugging(doc: NotebookDocument, cell?: NotebookCell) {
let dbg = this.notebookToDebugger.get(doc);
if (!dbg) {
dbg = new Debugger(doc);
dbg = new Debugger(doc, cell);
this.notebookToDebugger.set(doc, dbg);

try {
await dbg.session;

// toggle the breakpoint margin
void this.commandManager.executeCommand('notebook.toggleBreakpointMargin', doc);
if (!cell) {
// toggle the breakpoint margin
void this.commandManager.executeCommand('notebook.toggleBreakpointMargin', doc);
}
} catch (err) {
traceError(`Can't start debugging (${err})`);
void this.appShell.showErrorMessage(DataScience.cantStartDebugging());
Expand Down
Loading