Port pdb debugger to this repository #16
Conversation
| "languages": [ | ||
| "python" | ||
| ], | ||
| "type": "python-pdb-node", |
There was a problem hiding this comment.
Added a debug config so I could just test with a launch.json instead of a command.
| readonly context: vscode.ExtensionContext, | ||
| private readonly _ral: RAL | ||
| ) { | ||
| this._spawner = _ral.spawner.create(); |
There was a problem hiding this comment.
Debugger is using a 'Spawner' and 'Spawnee' abstraction. It needs only a few things from child_process.
There was a problem hiding this comment.
Once the WASI version is working, I hope this should be easy to implement there.
| * @param args: arguments to pass to the python | ||
| * @returns an object that can be listened to for stdout/stderr/stdin | ||
| */ | ||
| spawn(args: string[], cwd: string | undefined): Promise<Spawnee>; |
There was a problem hiding this comment.
I think? you can pass args to a worker? If not a message could be created to pass the args to pdb inside the worker.
Yes, in node but not in the browser. In the browser you need to decode them as a param string. See https://github.com/microsoft/vscode-wasm/blob/74b49f8c2df9ecb807a934ba0de86f6fb1db5b92/sync-api-common/src/browser/ril.ts#L74 and https://github.com/microsoft/vscode-wasm/blob/74b49f8c2df9ecb807a934ba0de86f6fb1db5b92/sync-api-common/src/node/ril.ts#L74 |
|
The code looks fine to me. I would only like to ask for one thing: if methods are private we should prefix them with the private keyword in TS and not only rely on the underscore. We don't have a clear rule in VS Code but most of the devs (including me) don't prefix private members with I propose that I add you as a committer to the repository so that you can merge the PR when approved. |
Sure sounds good. |
|
Code got merge from another PR. Closing |
Moved the code I had in microsoft/vscode-wasm#27 to this repository instead.
@dbaeumer I didn't wire up the web version of the debugger, and the desktop version is not using a WASI, but figured that would happen later once you get the new file descriptor setup.