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

Terminal should throw an event when a terminal has completed (and is waiting to close) #18376

Closed
Tyriar opened this issue Jan 10, 2017 · 9 comments
Assignees
Labels
terminal Integrated terminal issues
Milestone

Comments

@Tyriar
Copy link
Member

Tyriar commented Jan 10, 2017

From @dbaeumer:

an event when a task executed in a terminal has finished (e.g. the terminal process existed)

@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

What I actually want to do is the following:

this.terminalService.createInstance(`Task: ${task.name}`, undefined, ['/C', 'dir'], false);

dir is an example here and '/C' is currently specific to cmd.exe.

What I would like to see happening here is that the cmd.exe is started, dir is executed and then cmd.exe terminates. When cmd.exe terminates I would like to get an event.

Regarding the '/C' option: any change to have this abstracted in the terminal itself. Having that knowledge on the client side is not very nice.

@dbaeumer
Copy link
Member

What currently happens is the following:

  • the terminal is started
  • dir is not executed
  • the terminal stays alive

@dbaeumer
Copy link
Member

dbaeumer commented Jan 11, 2017

I changed it to
this.terminalService.createInstance(Task: ${task.name}, 'C:\\WINDOWS\\system32\\cmd.exe', ['/C', this.configuration.command], false); after some debugging but now no terminal shows up at all. I get an exception messageService.ts:126 write EPIPE

@Tyriar
Copy link
Member Author

Tyriar commented Jan 11, 2017

Did you call Terminal.show() immediately after? API terminals are launched in the background. You could try point to C:\\Windows\\sysnative\\cmd.exe and/or use an environment var to find C:\\Windows. Sysnative is a weird symlink thing that points to system32 or syswow64. Try that first and maybe rm -rf node_modules/node-pty node_modules/pty.js and try reinstall deps?

It would be good to encapsulate the bash and cmd command execution commands inside the TerminalService, however I'm not sure something like that should be exposed in the Terminal API. In general I don't want to encourage using a custom shell/args unless the specific use case calls for it (like tasks).

@dbaeumer
Copy link
Member

dbaeumer commented Jan 11, 2017

I got it working passing in true to waitOnExit. My code looks like this now:

let terminal = this.terminalService.createInstance(`Task: ${task.name}`, 'C:\\WINDOWS\\system32\\cmd.exe', ['/C', this.configuration.command], true);
this.terminalService.setActiveInstance(terminal);
this.terminalService.showPanel(false);

What I actually would like to have is a listener instead of the waitOnExit. But we already discussed this. Any advice on how to improve this?

@dbaeumer
Copy link
Member

Regarding:

It would be good to encapsulate the bash and cmd command execution commands inside the TerminalService, however I'm not sure something like that should be exposed in the Terminal API. In general I don't want to encourage using a custom shell/args unless the specific use case calls for it (like tasks).

As said we keep the task framework in the core for now. So it would allow us to experiment with some API here before thinking about making it official.

@Tyriar
Copy link
Member Author

Tyriar commented Jan 11, 2017

@dbaeumer I guess I'll expose some event on ITerminalInstance exit for now which when used with waitOnExit to allow reuse of the terminal (via another ITerminalInstance call to start up a new process while retaining the old output).

@Tyriar Tyriar removed the api label Jan 11, 2017
@Tyriar Tyriar closed this as completed in e0fb473 Jan 11, 2017
@Tyriar Tyriar changed the title Terminal API should throw an event when a terminal has completed (and is waiting to close) Terminal should throw an event when a terminal has completed (and is waiting to close) Jan 11, 2017
@Tyriar
Copy link
Member Author

Tyriar commented Jan 11, 2017

I closed this as it's possible internally now via ITerminalInstance.onExit

@hilleer
Copy link

hilleer commented Nov 16, 2017

Can you help me on how I get the state of the terminal - e.g. it finished running whatever command? I can't find any help in the documentation.

@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.
Labels
terminal Integrated terminal issues
Projects
None yet
Development

No branches or pull requests

3 participants