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.

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

support a "don't care" for cwd in the runInTerminal request #160

Closed
weinand opened this issue Nov 17, 2020 · 8 comments
Closed

support a "don't care" for cwd in the runInTerminal request #160

weinand opened this issue Nov 17, 2020 · 8 comments
Assignees
Labels
clarification Protocol clarification
Milestone

Comments

@weinand
Copy link
Contributor

weinand commented Nov 17, 2020

Since the cwd property of the runInTerminal request is not optional, it is not possible to express the "don't care" case, e.g. if a DA knows that the "cwd" is already correct and that there is no need to generate a cd ... command.

@weinand weinand added the feature-request Request for new features or functionality label Nov 17, 2020
@weinand weinand self-assigned this Nov 17, 2020
@puremourning
Copy link
Contributor

puremourning commented Nov 17, 2020

how about just sending . ?

A special "don't care" value is no less a breaking change than allowing the key to be omitted; both require a client capability, right?

@weinand
Copy link
Contributor Author

weinand commented Nov 17, 2020

A cwd . will still result in a cd .;
Please see dotnet/vscode-csharp#4203. Gregg wants to completely suppress the cd ...

@puremourning
Copy link
Contributor

not in my client. I don't really understand where the cd . is coming from... ? There's no requirement for the client to issue that as a command in the terminal.

@weinand
Copy link
Contributor Author

weinand commented Nov 17, 2020

Depending on the implementation of runInTerminal in the client, it might be necessary to ensure that the debuggee is launched in the specified cwd. For example if the terminal is reused across debug launches, and two projects have different working directories.

@gregg-miskelly
Copy link
Member

Both VS Code (so far I have tested Linux and Windows) and VS are happy with empty string. Anyone have a client which isn't?

@puremourning
Copy link
Contributor

In case it helps...

I suspect that any other client would reasonably expect the value supplied to be a valid path.

@weinand
Copy link
Contributor Author

weinand commented Nov 19, 2020

Since I don't want to introduce a new capability for this ancillary feature, I propose to change the cwd comment to the following:

  /**
   * Working directory for the command. For non-empty, valid paths this typically results in execution of a change directory command ('cd cwd'). 
   */
  cwd: string;

@gregg-miskelly
Copy link
Member

LGTM

@weinand weinand added this to the November 2020 milestone Nov 19, 2020
@weinand weinand added clarification Protocol clarification and removed feature-request Request for new features or functionality labels Nov 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification Protocol clarification
Projects
None yet
Development

No branches or pull requests

3 participants