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

Expose integrated terminal extension API #9957

Closed
Tyriar opened this issue Jul 29, 2016 · 19 comments · Fixed by #10635
Closed

Expose integrated terminal extension API #9957

Tyriar opened this issue Jul 29, 2016 · 19 comments · Fixed by #10635
Assignees
Labels
api feature-request Request for new features or functionality terminal Integrated terminal issues
Milestone

Comments

@Tyriar
Copy link
Member

Tyriar commented Jul 29, 2016

The initial extension API will allow:

  • Creating and optionally naming of a terminal instance
  • Sending commands (text sequences + '\n') to these "owned" terminal instances

Additional APIs will be considered via feedback from consumers.

Related: #547

@Tyriar Tyriar added feature-request Request for new features or functionality api terminal Integrated terminal issues labels Jul 29, 2016
@Tyriar Tyriar added this to the August 2016 milestone Jul 29, 2016
@Tyriar Tyriar self-assigned this Jul 29, 2016
@jrieken
Copy link
Member

jrieken commented Aug 5, 2016

please keep me in the loop

@Tyriar
Copy link
Member Author

Tyriar commented Aug 15, 2016

The current plan is to be able to create terminals and send text to them:

CreateTerminal(name?: string): Terminal
Terminal.runCommand(text: string): void

@weinand what sort of API would you expect for the debugging support which we eventually want (#10514)?

@jrieken
Copy link
Member

jrieken commented Aug 16, 2016

I would also suggest dispose, show, hide, name, like so

interface Terminal {
  name:string;
  runCommand(cmd:string)
  show(preserveFocus: boolean): void
  hide(): void;
  dispose(): void;
}

Also, I wonder if runCommand can return a promise that signals when execution is done, resolving to the exit code?

@weinand
Copy link
Contributor

weinand commented Aug 16, 2016

@Tyriar the proposed API seems to cover most of our needs.
Ideally it would be great if Terminal.runCommand() would return the process id of the command, but that's probably too tricky.
And a promise that signals when execution is done would be helpful too.

@Tyriar
Copy link
Member Author

Tyriar commented Aug 16, 2016

Thanks @weinand @jrieken, I'm not sure its possible to get the process ID as we're tracking the shell process, I'll look into whether we can get any info about the running process from the shell process.

@Tyriar
Copy link
Member Author

Tyriar commented Aug 16, 2016

There doesn't seem to be an easy way using pty.js to get the current process, however platform-specific options may exist. Given the process ID of the shell, we can get the child process(es) for it:

~
❯ ps aux | grep ./terminalProcess | head -n 1
daimms    6698  0.0  0.3 897696 42772 ?        Sl   05:48   0:00 /usr/share/code-insiders/code-insiders 

~
❯ ps aux | grep $(pgrep -P 6698) | head -n 1
daimms    6702  0.0  0.0  24628  6116 pts/19   Ss   05:48   0:00 /bin/bash

~
❯ ps aux | grep $(pgrep -P 6702) | head -n 1
daimms    8661 23.1  8.5 2147228 1041596 pts/19 Sl+ 06:00   0:37 gulp

@weinand
Copy link
Contributor

weinand commented Aug 16, 2016

On OS X I'm using a script that filters the output of psbased on the 'pty' (pseudo tty) of the terminal window and the command name:

ps -c -t $pty | awk '$4== $name { print $1 }'

@Tyriar
Copy link
Member Author

Tyriar commented Aug 16, 2016

I'm think sendText(text: string, addNewLine: boolean = true) may be better than runCommand as that more accurately describes what is actually happening, plus the optional parameter allows more flexibility.

@Tyriar
Copy link
Member Author

Tyriar commented Aug 16, 2016

@weinand that allows fetching of child processes on OS X?

@weinand
Copy link
Contributor

weinand commented Aug 16, 2016

Man page info about -t option of ps: "Display information about processes attached to the specified terminal devices".
So this is not exactly fetching the child processes... but it works fine for me.

@Tyriar
Copy link
Member Author

Tyriar commented Aug 16, 2016

I can only get process info about the currently running tty using -t, we would need to do on a separate process otherwise output would show to the user:

This gives what we want if run in the shell currently being run in:

❯ ps -t                                                                                                
  PID TTY      STAT   TIME COMMAND                                                                     
 8939 pts/20   Ss     0:00 /bin/bash                                                                   
12135 pts/20   R+     0:00 ps -t

We would want something like ps -t <where shell PID=...>, not sure if it's possible using -t currently.

@weinand
Copy link
Contributor

weinand commented Aug 16, 2016

On OS X the 't' option takes as an argument the name of a tty and I can retrieve that name from the new terminal. So I can run the ps as a separate process invisible to the user.

@Tyriar
Copy link
Member Author

Tyriar commented Aug 16, 2016

@weinand where name is something other than the PID? The proposed name param in the Terminal constructor is only a label for the dropdown currently.

@weinand
Copy link
Contributor

weinand commented Aug 16, 2016

@Tyriar assume I want to launch 'mono' in an external terminal as a debug target.
I need mono's process id because I need to kill it when debug sessions ends.

  • I create a terminal on OS.
  • I launch 'mono' in that terminal (but I cannot get the process id from this).

here is the pseudo code of what I do to find the pid of the mono process:

pty = terminal.tty    // get the tty device name from the terminal
name = "mono"    // the command that was launched in the terminal

// find all processes that were launched in the given terminal by the given command name
pid = ps -c -t $pty | awk '$4== $name { print $1 }'    // ideally this results in exactly one process id

@Tyriar
Copy link
Member Author

Tyriar commented Aug 19, 2016

Reopening as Terminal.hide() closes the panel regardless of whether the terminal is active or not (blocked on #10721).

@guogaigai
Copy link

Is there any terminal API provided for extension developers to create a terminal with the specified path as the initially start ,thank you @weinand

@jrieken
Copy link
Member

jrieken commented Aug 24, 2016

@Tyriar One more thing - we have an extension vscode-api-tests in which you can write tests against the (latest) API. They will be run as real extension with scripts/test-integration.sh.

@weinand
Copy link
Contributor

weinand commented Aug 24, 2016

@guogaigai please direct questions about the integrated terminal to @Tyriar. I'm only a client of that API.

Tyriar added a commit that referenced this issue Aug 25, 2016
@Tyriar
Copy link
Member Author

Tyriar commented Aug 29, 2016

Closing this off as it believe it's done, forking the test task to #11132

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api feature-request Request for new features or functionality terminal Integrated terminal issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants