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

Add shellPath to allow user specify preferred shell. Fix #10917 #11700

Merged
merged 2 commits into from
Sep 10, 2016

Conversation

octref
Copy link
Contributor

@octref octref commented Sep 8, 2016

No description provided.

@msftclas
Copy link

msftclas commented Sep 8, 2016

Hi @octref, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!


It looks like you're a Microsoft contributor (Zhenhang Wu). If you're full-time, we DON'T require a Contribution License Agreement. If you are a vendor, please DO sign the electronic Contribution License Agreement. It will take 2 minutes and there's no faxing! https://cla.microsoft.com.

TTYL, MSBOT;

@@ -3497,9 +3497,10 @@ declare namespace vscode {
* [Terminal.show](#Terminal.show) in order to show the terminal panel.
*
* @param name Optional human-readable string which will be used to represent the terminal in the UI.
* @param shellPath Path to the user-requested shell executable that will be used in the terminal
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change this to:

@param shellPath Optional path to a custom shell executable to be used in the terminal.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

@@ -85,8 +87,8 @@ export class ExtHostTerminalService {
this._proxy = threadService.get(MainContext.MainThreadTerminalService);
}

public createTerminal(name?: string): vscode.Terminal {
return new ExtHostTerminal(this._proxy, -1, name);
public createTerminal(name?: string, path?: string): vscode.Terminal {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

path -> shellPath

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.004%) to 61.834% when pulling 61ef6925d14bb59e9d310627066f77784b49c241 on octref/10917 into 3375a6d on master.

@octref
Copy link
Contributor Author

octref commented Sep 8, 2016

Updated to address comments.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.004%) to 61.834% when pulling 61ef6925d14bb59e9d310627066f77784b49c241 on octref/10917 into 3375a6d on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.007%) to 61.831% when pulling f85725f on octref/10917 into 3375a6d on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.006%) to 61.833% when pulling 9e410c2 on octref/10917 into 3375a6d on master.

@Tyriar
Copy link
Member

Tyriar commented Sep 10, 2016

I'm going to merge this as is without shellArgs as I'm in the middle of a big refactor and want to resolve the conflicts now rather than later. 👍

@Tyriar Tyriar merged commit c0c4029 into master Sep 10, 2016
@Tyriar Tyriar deleted the octref/10917 branch September 10, 2016 18:47
@octref
Copy link
Contributor Author

octref commented Sep 11, 2016

@Tyriar

Thanks. Should I do the shellArgs now or wait for your refactor?

@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants