Adds custom terminal launch settings by pflannery · Pull Request #3495 · microsoft/vscode

pflannery · 2016-02-26T14:55:04Z

This allows windows and linux users to specify which terminal they would like to launch when using the browser context menu -> "Open in Command Prompt".
Falls back to the default terminal if no settings specified.

New setting examples:

// Place your settings in this file to overwrite default and user settings.
"terminal.external": {
    "windowsExec": "cmd", // default
    "linuxExec": "x-terminal-emulator" // default for debian, otherwise "gnome-terminal" for gnome desktops
}

Ensures any spawn error is sent to the errors.onUnexpectedError event.

relates to #597

msftclas · 2016-02-26T14:55:07Z

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

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.

TTYL, MSBOT;

msftclas · 2016-02-26T14:58:52Z

@pflannery, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, MSBOT;

pflannery · 2016-02-26T15:02:53Z

A couple of things

~~I wasn't sure on was how I could add typings for this new setting.~~
~~I couldn't find any tests for this area so I haven't added a test. So instead I've extracted the dependencies use to spawn into spawnTerminal(...) so its easy to mock in the future~~

pflannery · 2016-03-15T21:17:40Z

@joaomoreno I've added a terminal configuration extension point for windows and linux which allows them to override the default shell launch by vscode.
I wasn't sure what to do for osx as it has script files, so I havent added anything for osx.

Tyriar · 2016-03-21T04:00:40Z

@pflannery any thoughts on how we could have fallback terminal emulators on Linux? We will eventually want to have at least 2 such as x-terminal-emulator (Debian) and gnome-terminal (Red Hat), probably more eventually as there are other display managers. I'd like to get it so that it works out of the box on Red Hat pretty soon (#4478).

I also noticed while doing a little research in to #4478 that gnome has something similar to the alternatives system in Debian for specifying a preferred terminal:

# Ubuntu
❯ gsettings get org.gnome.desktop.default-applications.terminal exec
'x-terminal-emulator'

# Fedora
❯ gsettings get org.gnome.desktop.default-applications.terminal exec
'gnome-terminal'

pflannery · 2016-03-21T04:25:23Z

@Tyriar could use process.env.DESKTOP_SESSION === 'gnome'

- ensures terminal default is used when the terminal setting is removed

pflannery · 2016-03-21T05:39:41Z

could have powershell as windows default?

Tyriar · 2016-03-21T07:08:47Z

@pflannery nice improvement! Ideally it would only revert to gnome-terminal if it's not Debian based, you could do this by checking if /etc/debian_version exists. Not sure if this is an OK thing to do for default values, but it probably only needs to be done once. @joaomoreno?

Tyriar · 2016-03-22T08:18:26Z

@pflannery this is more @joaomoreno's area, but powershell isn't built into older versions of Windows like 7 is it?

pflannery · 2016-03-22T16:26:06Z

@Tyriar just looked this up and looks like it's vista backwards that doesn't support powershell out of the box - https://en.wikipedia.org/wiki/Windows_PowerShell#PowerShell_2.0

Tyriar · 2016-03-24T01:40:03Z

src/vs/workbench/parts/execution/electron-browser/terminalService.ts

+	private spawnTerminal(spawner, configuration, path: string, onExit, onError) {
+		let terminalConfig = configuration.terminal;
+		let exec = terminalConfig.linux.exec || defaultLinuxTerm;
+		const child = spawner.spawn(exec, [], { cwd: path });


Does just setting the cwd work with gnome-terminal? You may need to pass in --working-directory=path for that?

node runs gnome-terminal at cwd specified in the options. I tested on Fedora 23 and it does go to the cwd specified. I think it's just a useful flag when someone wants to run from a different path other than the cwd gnome-terminal was executed from.

joaomoreno · 2016-03-24T07:26:16Z

Currently OOF, will check this next week.

A standard Fedora install comes with 2 multiple line environment variables. Since `env` was previously split by '\n' this would break them, causing errors in the output pane and in terminals launched through the file explorer (see #3495). Fixes #3928

A standard Fedora install comes with 2 multiple line environment variables. Since `env` was previously split by '\n' this would break them, causing errors in the output pane and in terminals launched through the file explorer (see #3495). The original commit didn't work on OSX since `env` does not support the --null arg. This version can fail if a command line arg's 1+th line looks like an environment variable. There is no easy way to prevent this since `process.env` cannot be leveraged. Since the likelyhood of this happening is small, plus the chance of it causing any significant issue is also small it's a reasonable compromise for the time being. Fixes #3928 Fixes #4672

Tyriar · 2016-04-04T14:57:29Z

Ping @joaomoreno

Tyriar · 2016-04-18T16:37:36Z

src/vs/workbench/parts/execution/common/terminal.ts

+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+import fs = require('fs');
+import env = require('vs/base/common/platform');


env -> platform, I think the instances where this is env are older ones that weren't cleaned up

Tyriar · 2016-04-18T18:50:35Z

src/vs/workbench/parts/execution/electron-browser/terminal.ts

 }

-export const defaultWindowsTerm = 'cmd';
+export const DEFAILT_WINDOWS_TERM = 'cmd';


Typo DEFAULT

Tyriar · 2016-04-18T19:11:53Z

src/vs/workbench/parts/execution/test/browser/terminalService.test.ts

@@ -0,0 +1,129 @@
+/*---------------------------------------------------------------------------------------------


This should also live in electron-browser/.

Tyriar · 2016-04-18T19:12:43Z

I think this is good to go other than moving the test to electron-browser/ and the DEFAULT typo.

Tyriar · 2016-04-18T20:09:30Z

Love it 😃 Thanks @pflannery! 🎆

pflannery · 2016-04-18T20:20:33Z

thanks!

willin · 2016-04-22T18:27:56Z

+1 https://atom.io/packages/terminal-panel

trendoid · 2016-05-10T12:26:26Z

I'm just getting to use this today. Thanks @pflannery I love it.

msftclas added the cla-required label Feb 26, 2016

msftclas added cla-signed and removed cla-required labels Feb 26, 2016

joaomoreno self-assigned this Mar 1, 2016

pflannery mentioned this pull request Mar 9, 2016

Request feature "Integrated terminal" #143

Closed

adds terminal command configuration for windows and linux

2695f19

pflannery changed the title ~~Adds custom terminal launch settings for win~~ Adds custom terminal launch settings Mar 15, 2016

This was referenced Mar 19, 2016

OS X: "Open in Terminal" doesn't work for me #4298

Closed

"Open in Terminal" not working on Fedora #4478

Closed

makes gnome-terminal default when running on gnome desktop

abe8d12

- ensures terminal default is used when the terminal setting is removed

adds debian check for default terminal

aa17b93

adds TerminalService tests

aa3f8d7

Tyriar reviewed Mar 24, 2016
View reviewed changes

pflannery mentioned this pull request Mar 24, 2016

VSCode corrupts multi-line environment variables #3928

Closed

joaomoreno added this to the April 2016 milestone Apr 4, 2016

joaomoreno mentioned this pull request Apr 4, 2016

Customizable 'Open in Command Prompt' console #4848

Closed

Tyriar assigned Tyriar and unassigned joaomoreno Apr 15, 2016

joaomoreno mentioned this pull request Apr 18, 2016

do i have a way to change native windows console ? #5421

Closed

Tyriar reviewed Apr 18, 2016
View reviewed changes

Response to ALL_CAPS_NAMING and code org.

c84a715

Tyriar reviewed Apr 18, 2016
View reviewed changes

Response to typo

ef72b65

Tyriar reviewed Apr 18, 2016
View reviewed changes

pflannery added 2 commits April 18, 2016 20:16

Response to configuration property name change

5d46df9

Response to move test to electron-browser folder

58317c2

Tyriar merged commit dadae2b into microsoft:master Apr 18, 2016

pflannery deleted the open-custom-win-terminal branch April 18, 2016 20:20

This was referenced Apr 18, 2016

Support custom terminal launch settings on OS X #5462

Closed

Test: Custom terminal launch settings #5463

Closed

Tyriar mentioned this pull request May 17, 2016

Improve Open in Terminal in Mac #6136

Merged

github-actions bot locked and limited conversation to collaborators Mar 27, 2020

		@@ -0,0 +1,129 @@
		/*---------------------------------------------------------------------------------------------

Conversation

pflannery commented Feb 26, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

msftclas commented Feb 26, 2016

Uh oh!

msftclas commented Feb 26, 2016

Uh oh!

pflannery commented Feb 26, 2016

Uh oh!

pflannery commented Mar 15, 2016

Uh oh!

Tyriar commented Mar 21, 2016

Uh oh!

pflannery commented Mar 21, 2016

Uh oh!

pflannery commented Mar 21, 2016

Uh oh!

Tyriar commented Mar 21, 2016

Uh oh!

Tyriar commented Mar 22, 2016

Uh oh!

pflannery commented Mar 22, 2016

Uh oh!

Tyriar Mar 24, 2016

Choose a reason for hiding this comment

Uh oh!

pflannery Mar 24, 2016

Choose a reason for hiding this comment

Uh oh!

joaomoreno commented Mar 24, 2016

Uh oh!

Tyriar commented Apr 4, 2016

Uh oh!

Tyriar Apr 18, 2016

Choose a reason for hiding this comment

Uh oh!

Tyriar Apr 18, 2016

Choose a reason for hiding this comment

Uh oh!

Tyriar Apr 18, 2016

Choose a reason for hiding this comment

Uh oh!

Tyriar commented Apr 18, 2016

Uh oh!

Tyriar commented Apr 18, 2016

Uh oh!

pflannery commented Apr 18, 2016

Uh oh!

willin commented Apr 22, 2016

Uh oh!

trendoid commented May 10, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

pflannery commented Feb 26, 2016 •

edited

Loading