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

Integrated terminal: Shell args not honored on Windows #8429

Closed
chrmarti opened this Issue Jun 28, 2016 · 13 comments

Comments

Projects
None yet
7 participants
@chrmarti
Contributor

chrmarti commented Jun 28, 2016

Testing #8222

  • VSCode Version: Code - Insiders 1.3.0-insider (d0c2b89, 2016-06-28T05:07:32.441Z)
  • OS Version: Windows_NT ia32 6.3.9600

Steps to Reproduce:

  1. Using the following settings:
    "terminal.integrated.shell.windows": "E:\cygwin64\bin\bash.exe",
    "terminal.integrated.shellArgs.windows": ["-v"]
  2. Bash should echo all lines read, open terminal and run: echo 123
  3. "echo 123" isn't echoed (only "123" is)

@Tyriar Tyriar self-assigned this Jun 29, 2016

@Tyriar Tyriar added this to the June 2016 milestone Jun 29, 2016

@Tyriar

This comment has been minimized.

Show comment
Hide comment
@Tyriar

Tyriar Jun 29, 2016

Member

Going to investigate for June, may not be able to get a fix though.

Member

Tyriar commented Jun 29, 2016

Going to investigate for June, may not be able to get a fix though.

@Tyriar

This comment has been minimized.

Show comment
Hide comment
@Tyriar

Tyriar Jun 30, 2016

Member

Upstream issue: chjj/pty.js#137, going to remove the config for the time being.

Member

Tyriar commented Jun 30, 2016

Upstream issue: chjj/pty.js#137, going to remove the config for the time being.

@Tyriar Tyriar added the upstream label Jun 30, 2016

Tyriar added a commit that referenced this issue Jun 30, 2016

Remove windows shellArgs config
This doesn't actually work due to an upstream issue

Related #8429

@Tyriar Tyriar modified the milestones: Backlog, June 2016 Jun 30, 2016

@Bigous

This comment has been minimized.

Show comment
Hide comment
@Bigous

Bigous Jul 12, 2016

Contributor

just to help,
The issue is that on windows there is no shellArgs on settings (linux and osx have).
version 1.3.1 is still lacking this option.

Contributor

Bigous commented Jul 12, 2016

just to help,
The issue is that on windows there is no shellArgs on settings (linux and osx have).
version 1.3.1 is still lacking this option.

@Bigous

This comment has been minimized.

Show comment
Hide comment
@Bigous

Bigous Jul 12, 2016

Contributor

One can use bash from git too so you can test it without having to install cygwin.

and test with the parameters:

    // The path of the shell that the terminal uses on Windows.
    "terminal.integrated.shell.windows": "C:\\Program Files\\Git\\usr\\bin\\bash.exe",
    "terminal.integrated.shellArgs.windows": ["--login", "-i"],

it shoul'd open the bash with your windows profile loaded (with environment variables set among other thins).
try to enter the command ls and see if it returns the list of files.

Contributor

Bigous commented Jul 12, 2016

One can use bash from git too so you can test it without having to install cygwin.

and test with the parameters:

    // The path of the shell that the terminal uses on Windows.
    "terminal.integrated.shell.windows": "C:\\Program Files\\Git\\usr\\bin\\bash.exe",
    "terminal.integrated.shellArgs.windows": ["--login", "-i"],

it shoul'd open the bash with your windows profile loaded (with environment variables set among other thins).
try to enter the command ls and see if it returns the list of files.

@Tyriar

This comment has been minimized.

Show comment
Hide comment
@Tyriar

Tyriar Jul 12, 2016

Member

@Bigous I removed the setting as it doesn't work in the library we're using. So this needs to be fixed in pty.js before we can add it back.

Member

Tyriar commented Jul 12, 2016

@Bigous I removed the setting as it doesn't work in the library we're using. So this needs to be fixed in pty.js before we can add it back.

@Bigous

This comment has been minimized.

Show comment
Hide comment
@Bigous

Bigous Jul 28, 2016

Contributor

Workaround: Create a batch file and set it as your windows shell. Mine is this.

Contributor

Bigous commented Jul 28, 2016

Workaround: Create a batch file and set it as your windows shell. Mine is this.

@the-ress

This comment has been minimized.

Show comment
Hide comment
@the-ress

the-ress Oct 10, 2016

Contributor

I've created a pull request with the fix here: chjj/pty.js#175

Contributor

the-ress commented Oct 10, 2016

I've created a pull request with the fix here: chjj/pty.js#175

@Tyriar

This comment has been minimized.

Show comment
Hide comment
@Tyriar

Tyriar Oct 11, 2016

Member

@the-ress awesome! I'll test this out on vscode's fork soon and if all works well it should land in v1.7.

Member

Tyriar commented Oct 11, 2016

@the-ress awesome! I'll test this out on vscode's fork soon and if all works well it should land in v1.7.

@Tyriar Tyriar modified the milestones: October 2016, Backlog Oct 11, 2016

Tyriar added a commit that referenced this issue Oct 12, 2016

Uplevel pty.js
Fixes #7727
Part of #8429

@Tyriar Tyriar closed this in 52690ed Oct 12, 2016

@Tyriar

This comment has been minimized.

Show comment
Hide comment
@Tyriar

Tyriar Oct 12, 2016

Member

Fixed thanks to @the-ress's pull request! You can try this out in the Insiders build tomorrow or in stable v1.7.

Member

Tyriar commented Oct 12, 2016

Fixed thanks to @the-ress's pull request! You can try this out in the Insiders build tomorrow or in stable v1.7.

@daviwil

This comment has been minimized.

Show comment
Hide comment
@daviwil

daviwil Oct 12, 2016

Contributor

That is awesome news, will save me a ton of trouble. Thanks a lot @the-ress!

Contributor

daviwil commented Oct 12, 2016

That is awesome news, will save me a ton of trouble. Thanks a lot @the-ress!

@daviwil

This comment has been minimized.

Show comment
Hide comment
@daviwil

daviwil Oct 14, 2016

Contributor

This is working perfectly for me now with the Terminal API in today's Insiders build. Thanks again @the-ress!

Contributor

daviwil commented Oct 14, 2016

This is working perfectly for me now with the Terminal API in today's Insiders build. Thanks again @the-ress!

@chrmarti chrmarti added the verified label Oct 27, 2016

@bagofmice

This comment has been minimized.

Show comment
Hide comment
@bagofmice

bagofmice Jun 10, 2017

I notice that this is still a problem for terminals on windows in 1.13. I've created a batch file workaround in an extension here:
https://marketplace.visualstudio.com/items?itemName=bagonaut.mongogo

bagofmice commented Jun 10, 2017

I notice that this is still a problem for terminals on windows in 1.13. I've created a batch file workaround in an extension here:
https://marketplace.visualstudio.com/items?itemName=bagonaut.mongogo

@wycats

This comment has been minimized.

Show comment
Hide comment
@wycats

wycats Sep 4, 2017

I've hit this issue on the latest stable (1.15) and reverted back to cmd as my default shell. Any idea why it was believed to be solved but regressed?

wycats commented Sep 4, 2017

I've hit this issue on the latest stable (1.15) and reverted back to cmd as my default shell. Any idea why it was believed to be solved but regressed?

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 17, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.