Skip to content
This repository was archived by the owner on Nov 30, 2023. It is now read-only.

Remove pre-set terminal setting once 1.57 is released #860

Merged
merged 3 commits into from
Jun 1, 2021

Conversation

Chuxel
Copy link
Member

@Chuxel Chuxel commented May 13, 2021

This PR removes the "terminal.integrated.shell.linux" setting that was in place due to the fact that shells like zsh are often not in containers. Previously in this scenario, a user setting would cause the terminal not to appear.

The new terminal "profile" capability in VS Code solves this by falling back to either $SHELL or the shell in /etc/passwd if the specified profile fails as of the upcoming VS Coe 1.57 release (currently in insiders).

Related: microsoft/vscode#123157, #838

Copy link
Member

@Tyriar Tyriar left a comment

Choose a reason for hiding this comment

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

It would be a good idea to verify this in containers before you push this

@@ -9,9 +9,7 @@
},

// Set *default* container specific settings.json values on container create.
"settings": {
"terminal.integrated.shell.linux": null
Copy link
Member

Choose a reason for hiding this comment

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

What was this meant to accomplish? Overriding the user setting in case it's not installed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct. The fallback eliminates the need for it.

@Chuxel
Copy link
Member Author

Chuxel commented May 13, 2021

It would be a good idea to verify this in containers before you push this

@Tyriar Yep - this gives us an easy PR to test them. Given 1.56 just came out, we can hold a bit before merging. I did see this working in VS Code Insiders 1.57, but 1.56.1 didn't seem to pick up updated the setting when I tried it. (Its set in the PowerShell definition since the point of that one is to use powershell. 😄 ) So I assume it and the fallback aren't there yet.

@Chuxel
Copy link
Member Author

Chuxel commented Jun 1, 2021

Merging in prep for 1.57 endgame week. FYI @chrmarti

@Chuxel Chuxel merged commit 1bdd958 into main Jun 1, 2021
@Chuxel Chuxel deleted the clantz/change-terminal-setting branch June 9, 2021 21:44
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.

2 participants