Skip to content

dap: Ensure cwd is present in runInTerminal request #1915

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

osiewicz
Copy link

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

@osiewicz osiewicz requested a review from a team as a code owner June 24, 2025 22:41
@rchiodo
Copy link
Contributor

rchiodo commented Jun 24, 2025

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
Copy link
Author

osiewicz commented Jun 24, 2025

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
Contributor

rchiodo commented Jun 24, 2025

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
Contributor

rchiodo commented Jun 24, 2025

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@rchiodo
Copy link
Contributor

rchiodo commented Jun 24, 2025

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 free to 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