Skip to content

dap: Ensure cwd is present in runInTerminal request - #1915

Open
Piotr Osiewicz (osiewicz) wants to merge 1 commit into
microsoft:mainfrom
osiewicz:dap-run-in-terminal-non-empty-cwd
Open

dap: Ensure cwd is present in runInTerminal request#1915
Piotr Osiewicz (osiewicz) wants to merge 1 commit into
microsoft:mainfrom
osiewicz:dap-run-in-terminal-non-empty-cwd

Conversation

@osiewicz

Copy link
Copy Markdown
Contributor

Per https://microsoft.github.io/debug-adapter-protocol/specification, runInTerminalRequestArguments.cwd is a mandatory property (although it can be left empty).

@osiewicz
Piotr Osiewicz (osiewicz) requested a review from a team as a code ownerJune 24, 2025 22:41
@rchiodo

Copy link
Copy Markdown
Contributor

Is this breaking something else? I'd be wary of making it '' or setting it at all given that VS code works with it without the parameter.

@osiewicz

Piotr Osiewicz (osiewicz) commented Jun 24, 2025

Copy link
Copy Markdown
ContributorAuthor

Yes, for one we've had to work around it in Zed in our deserialization code for that request type.
Edit: At a cursory glance, Helix might break with unfilled CWD as well:
https://github.com/helix-editor/helix/blob/master/helix-dap/src/types.rs#L756
Haven't tested that though.

@rchiodo

Copy link
Copy Markdown
Contributor

I think VS code is using it here:
https://github.com/microsoft/vscode/blob/d8ffc540d19fd4f936c99dd1cc38a64d3a28a079/src/vs/workbench/api/node/extHostDebugService.ts#L103, so when it's undefined it may have different behavior than if it's just empty. It'd need to be tested.

@rchiodo

Copy link
Copy Markdown
Contributor

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@rchiodo

Copy link
Copy Markdown
Contributor

It looks like it makes the tests fail at least:

D+00000.080: Client[1] --> {
"seq": 3,
"type": "response",
"request_seq": 7,
"success": false,
"command": "runInTerminal",
"message": "[Errno 2] No such file or directory: ''"
}

So the tests would have to be updated to ignore an empty cwd.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@osiewicz@rchiodo