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

Expose a terminal's creation options as API #63052

Closed
DonJayamanne opened this issue Nov 13, 2018 · 16 comments · Fixed by #84005
Closed

Expose a terminal's creation options as API #63052

DonJayamanne opened this issue Nov 13, 2018 · 16 comments · Fixed by #84005
Assignees
Labels
api api-finalization feature-request Request for new features or functionality terminal Integrated terminal issues verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@DonJayamanne
Copy link
Contributor

Right now when a terminal is created the onDidOpenTerminal event fires providing the ref to the terminal created.

This information is crucial for the Python Extension when activating the Python Environment in the terminal.

Note: Each workspace folder can have different python environments, hence the workspace folder Uri is necessary when identifying the python environment that needs to be activated.

Suggestion: Add workspaceUr or cwd to the Terminal object.

@Tyriar
Copy link
Member

Tyriar commented Nov 30, 2018

Well terminals aren't bound to workspaces and we can't expose a dynamic cwd, how about this?

export interface Terminal {
  readonly initialCwd: string;
}

@Tyriar Tyriar added feature-request Request for new features or functionality info-needed Issue requires more information from poster labels Nov 30, 2018
@DonJayamanne
Copy link
Contributor Author

That's perfect, yes I agree with your comment.

@Tyriar Tyriar removed the info-needed Issue requires more information from poster label Dec 3, 2018
@Tyriar
Copy link
Member

Tyriar commented Oct 8, 2019

@DonJayamanne if #46696 worked in multi-root workspaces would that also fix the problem?

@Tyriar Tyriar added info-needed Issue requires more information from poster under-discussion Issue is under discussion for relevance, priority, approach and removed feature-request Request for new features or functionality labels Oct 8, 2019
@DonJayamanne
Copy link
Contributor Author

Unfortunately no.

@Tyriar
Copy link
Member

Tyriar commented Oct 9, 2019

@DonJayamanne you need to do more than just set the python version on the path, like run some script to set up virtual env or something?

@DonJayamanne
Copy link
Contributor Author

like run some script to set up virtual env or something?

Yes, thats right.

@Tyriar Tyriar added api api-proposal feature-request Request for new features or functionality and removed info-needed Issue requires more information from poster under-discussion Issue is under discussion for relevance, priority, approach labels Oct 9, 2019
@Tyriar Tyriar added this to the October 2019 milestone Oct 9, 2019
@Tyriar
Copy link
Member

Tyriar commented Oct 9, 2019

Adding to October for discussion, this won't be implemented in October.

@Tyriar
Copy link
Member

Tyriar commented Oct 14, 2019

I discussed this at the API meeting today, instead of exposing an initialCwd property on Terminal we could make it more generic and just expose the options object used to create the terminal:

export interface Terminal {
	/**
	 * The object used to initialize the terminal, this is useful for things like detecting the
	 * shell type of shells not launched by the extension or detecting what folder the shell was
	 * launched in.
	 */
	readonly creationOptions: Readonly<TerminalOptions | ExtensionTerminalOptions>;
}

@Tyriar
Copy link
Member

Tyriar commented Nov 4, 2019

We should use Readonly<...>, probably be a good idea to freeze the object behind the scenes too

@Tyriar Tyriar reopened this Nov 29, 2019
@Tyriar Tyriar changed the title Provide workspace Uri when a terminal has been created Expose a terminal's creation options as API Nov 29, 2019
@Tyriar Tyriar modified the milestones: November 2019, On Deck Nov 29, 2019
@Tyriar
Copy link
Member

Tyriar commented Nov 29, 2019

@DonJayamanne I'd like some feedback/validation before promoting this to stable, if the idea is validated then we can look at promoting it to stable for December.

@Tyriar Tyriar modified the milestones: On Deck, December 2019 Nov 29, 2019
@DonJayamanne
Copy link
Contributor Author

Woo hoo. Perfect. Validated, thanks

@Tyriar
Copy link
Member

Tyriar commented Jan 27, 2020

To verify:

  1. Create an extension that logs creationOptions for each Terminal in vscode.window.terminals whenever a terminal is created via onDidOpenTerminal
  2. Create some terminals using normal means, createTerminal and another extension and verify creationOptions looks correct.

@connor4312 connor4312 added the verified Verification succeeded label Jan 29, 2020
@connor4312
Copy link
Member

I tried this. I do see the creation options, but the cwd is undefined when creating a terminal via the + button, in both single root and multiroot workspaces. I would expect this to be set to the relevant workspace folder.

@connor4312 connor4312 added the verification-found Issue verification failed label Jan 29, 2020
@Tyriar
Copy link
Member

Tyriar commented Jan 30, 2020

@connor4312 are you sure it's not working in multi-root workspaces? It's undefined in single root workspaces for me, which means open the default cwd, but for multi-root it always sets it as a URI? Are you by chance using this command?

Screen Shot 2020-01-30 at 4 57 46 AM

It is expected to come through as undefined for that as well.

@Tyriar Tyriar removed verification-found Issue verification failed verified Verification succeeded labels Jan 30, 2020
@connor4312
Copy link
Member

Okay, if that's expected then I think we're good 🙂

@connor4312 connor4312 added the verified Verification succeeded label Jan 30, 2020
@Tyriar
Copy link
Member

Tyriar commented Jan 30, 2020

Thanks, it is an important edge case for extensions to consider, my hope is that it's understood as those terminals were created just like a call to createTerminal when cwd is undefined.

@vscodebot vscodebot bot locked and limited conversation to collaborators Feb 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api api-finalization feature-request Request for new features or functionality terminal Integrated terminal issues verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants