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

Allow terminals to be reused #18377

Closed
Tyriar opened this issue Jan 10, 2017 · 8 comments · Fixed by #18586
Closed

Allow terminals to be reused #18377

Tyriar opened this issue Jan 10, 2017 · 8 comments · Fixed by #18586
Assignees
Labels
terminal Integrated terminal issues
Milestone

Comments

@Tyriar
Copy link
Member

Tyriar commented Jan 10, 2017

From @dbaeumer:

Reuse of a terminal. Not necessarily the terminal process more the terminal pane. If the user executes a task and the task finishes and runs the task again it would be cool if the output of the task could show up in the same terminal. Constantly pressing a key to close and old terminal might not be very intuitive.

@Tyriar Tyriar added api terminal Integrated terminal issues labels Jan 10, 2017
@Tyriar Tyriar added this to the January 2017 milestone Jan 10, 2017
@Tyriar Tyriar self-assigned this Jan 10, 2017
@dbaeumer
Copy link
Member

For January we decided to keep the task in the core. So there is no need to make this 'public' API now. May be it would be good to first experiment a little with the new API.

@Tyriar
Copy link
Member Author

Tyriar commented Jan 11, 2017

@dbaeumer just to make sure, were you thinking of using a waitOnExit prompt (press any key to close) that can be reused? Or a terminal that executes something, the pty process dies and just sits there ("dead") until another task command is sent?

@dbaeumer
Copy link
Member

@Tyriar I would like to achieve the following:

  • create a new terminal and allow terminal reuse (could be an additional option) without actually running a terminal right away (if necessary we could run it as well)
  • run a command in the terminal. This will create the actual shell process. Usually the shell would be started with /c
  • receive an exit event when the shell exists
  • reuse the terminal to run another command in the terminal which creates a new shell process.

Use case if for a build task: if the user runs the build tasks 3 times in a row I would like to have one terminal (e.g. on entry in the drop down in the terminal pane) which contains the output of all three task runs.

@dbaeumer
Copy link
Member

@Tyriar I have one additional question. Does the terminal require to run a shell or would the terminal be able to run any other arbitrary process (for example a node.exe).

@dbaeumer
Copy link
Member

@Tyriar I got it working to run an arbitrary program. Only drawback is that I needed to specify the full path to the program (e.g. "C:\\Program Files\\nodejs\\node.exe"). Any idea why this is the case?

@Tyriar
Copy link
Member Author

Tyriar commented Jan 12, 2017

@dbaeumer do you mean the shell path you're using is node and it's not working like that? I guess the PATH resolution stuff is contained within cmd, I would think it would probably be safest to always wrap the command within cmd /c or sh -c.

Wrapping the command means the process will die when it finishes so I don't see a downside?

@dbaeumer
Copy link
Member

@Tyriar the only problem is argument passing with spaces contained. If you call node directly then you pass the args as a string[]. If you call the cmd.exe for example you need to construct a single string with proper arg quoting to ensure everything works when for example the command to call contains spaces.

And you save a process :-)

@gandhis1
Copy link

@DonJayamanne could this be useful for Python terminal execution?

@Tyriar Tyriar removed the api label Jan 23, 2017
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
terminal Integrated terminal issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants