Use in-memory debug adapter instead of spinning up new process #2672
Conversation
| private static readonly HEADER_LINESEPARATOR = /\r?\n/; // allow for non-RFC 2822 conforming line separators | ||
| private static readonly HEADER_FIELDSEPARATOR = /: */; | ||
|
|
||
| private _rawData = Buffer.allocUnsafe(0); |
There was a problem hiding this comment.
What's the initialisation here used for? I'm guessing there's somewhere we can pass in undefined and this is the cheapest way to pass in an empty buffer?
There was a problem hiding this comment.
Honestly, this comes from VS Code's implementation that's quite challenging to fully understand... they also set this in the constructor... so I'm really unsure why.
src/debugAdapter.ts
Outdated
| onDidSendMessage: Event<DebugProtocolMessage> = this._sendMessage.event; | ||
|
|
||
| constructor(namedPipe: string, logger: Logger) { | ||
| this._debugServiceSocket = connect(namedPipe); |
There was a problem hiding this comment.
I worry a bit about objects that can't be constructed without connecting to an IPC stream, but really it's probably fine
There was a problem hiding this comment.
now the constructor does almost nothing and start() method does the connection.
| private _isConnected: boolean = false; | ||
| private _debugMessageQueue: DebugProtocolMessage[] = []; | ||
|
|
||
| private _debugServiceSocket: Socket; |
There was a problem hiding this comment.
Could these be added to the constructor for injection-initialisation?
There was a problem hiding this comment.
decided against this because I wanted the _ prefix.
PR Summary
This leverages a newish feature in VS Code called a
DebugAdaptertype. I heard about it from this issue:microsoft/vscode#84643 (comment)
This gives us:
This partially solves #2358 because now, the debug adapter won't die... it'll kick the extension on and say "run it again in a bit once the extension fully starts"
PR Checklist
Note: Tick the boxes below that apply to this pull request by putting an
xbetween the square brackets.Please mark anything not applicable to this PR
NA.WIP:to the beginning of the title and remove the prefix when the PR is ready