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 more information about the state of the integrated terminal internally within vscode and to the API #11422

Closed
Tyriar opened this issue Sep 1, 2016 · 19 comments
Assignees
Labels
api feature-request Request for new features or functionality terminal Integrated terminal issues
Milestone

Comments

@Tyriar
Copy link
Member

Tyriar commented Sep 1, 2016

Originally discussed in #9957 and then in #11214, consumers of the API want to know more about the process(es) currently being run within the terminal, such as what process(es) are running and whether the terminal is "busy".

@Tyriar Tyriar added feature-request Request for new features or functionality api terminal Integrated terminal issues labels Sep 1, 2016
@Tyriar Tyriar added this to the Backlog milestone Sep 1, 2016
@Tyriar Tyriar self-assigned this Sep 1, 2016
@daviwil
Copy link
Contributor

daviwil commented Sep 9, 2016

Specifically I'd like to request that the PID of the terminal be available on the Terminal object. Some of the coordination I do in the PowerShell extension relies on having the PID of the PowerShell process so that my debug adapter knows where it should look for connection details when a debugging session is started.

@Tyriar
Copy link
Member Author

Tyriar commented Sep 9, 2016

@daviwil what sort of things are you going to use the PID for? Wondering if we can provide more high level APIs then just the PID to prevent some boilerplate in many extensions.

@daviwil
Copy link
Contributor

daviwil commented Sep 9, 2016

That's a good question, and typing out my reasoning just now reminded me that I don't actually use the PowerShell PID for anything other than just logging purposes (and knowing which process to attach a debugger to when things go south). I guess it isn't super critical for me to have the PID but it's definitely helpful for debugging purposes.

@wclr
Copy link

wclr commented Sep 10, 2016

@daviwil

Specifically I'd like to request that the PID of the terminal be available on the Terminal object.

It is now still on terminal._id

@Tyriar

Wondering if we can provide more high level APIs then just the PID to prevent some boilerplate in many extensions.

It would be nice if availability of PID could be preserved at least before that moment.

@Tyriar
Copy link
Member Author

Tyriar commented Sep 10, 2016

@whitecolor as I mentioned in #10546 (comment), terminal,_id is a private API that is very likely going away in v1.6.

@Tyriar
Copy link
Member Author

Tyriar commented Sep 13, 2016

Forked exposing PID to #11919 which I hope to get in for September.

@Tyriar
Copy link
Member Author

Tyriar commented Oct 7, 2016

From @isidorn in #13351 (comment)

Currently we support having a preLaunchTask specified in 'launch.json' to run a task before a user starts debugging (more details here). There are requests that we support running a preLaunchTask even if a user does not have 'tasks.json' setup.

My proposal for this is that the user could specify a full command in his 'launch.json' which we would just forward to our terminal via the send api. The only missing piece for this would be for the terminal to be aware when a command has finished running - or when it becomes idle. Currently this problem is solved in the task framewrok by @dbaeumer so maybe we could do something similar in our terminal.

I think @jrieken might be also interested in something similar for the passing of errors from our terminal to the marker service.

@Tyriar let me know what you think and if this request makes sense
@weinand fyi

@weinand and I have spoken about it before. It's a little more tricky that just saying that some text passed to Terminal.sendText will spawn a process. It could be input to a process already running in the shell (eg. vim), it could be a partial command that expects more input (eg git commit -m "blah\n) and so on.

One idea is to track when child processes of the pty process are spawned and die and report that, not sure how difficult/reliable that is though, it would probably also be best done upstream.

Another is to ask the terminal whether it has a particular child process after the command has started, but this still has the reliability issues mentioned earlier.

Related: #7321
Related: #13337

@Tyriar Tyriar changed the title Expose more information to the API about the state of the integrated terminal Expose more information about the state of the integrated terminal internally within vscode and to the API Oct 7, 2016
@isidorn
Copy link
Contributor

isidorn commented Oct 10, 2016

@Tyriar thanks for providing more details
@dbaeumer did we already solve this problem for the tasks? How do you know when a task is inactive or when it stops? Can we reuse this logic in the terminal land?

@dbaeumer
Copy link
Member

This is done by pattern matching. If you define a watch tasks you can provide patterns for the inactive->active transition and active->inactive transition.

@Tyriar
Copy link
Member Author

Tyriar commented Nov 14, 2016

I wonder if checking creating some events in xterm.js that trigger when the cursor visibility toggles might be a better way of getting whether the shell is busy. I think there are issues on Windows with the cursor displaying sometimes when it shouldn't but if that was fixed it might be a reasonable way of doing this?

@Tyriar
Copy link
Member Author

Tyriar commented Nov 14, 2016

Other ideas:

  • Wrapping commands using something like preexec/postexec on zsh to write a lock file or something (see https://github.com/rcaloras/bash-preexec). Cross-platform and full shell compatibility would probably no be possible though.
  • Investigate xterm.js code/state information whether anything useful can be exposed

@jrieken
Copy link
Member

jrieken commented Nov 15, 2016

Pretending I didn't mention already the concept of co-processes in iterm. See https://iterm2.com/documentation-coprocesses.html and #13337

@Tyriar
Copy link
Member Author

Tyriar commented Nov 15, 2016

@jrieken co-processes wouldn't allow an extension to determine if a process is currently running the shell though, only reacting to stdout. While you could potentially scan stdout for a pattern to know if the command has terminated, if there's a way to solve this in a more generic way that was guaranteed to work would be very useful and convenient. Something like this:

Terminal.isAcceptingInput(): boolean;
// isBusy, canInput, isProcessRunning, etc.

enum TerminalState {
  BUSY,
  NOT_BUSY
}
Terminal.onStateChange(): Event<TerminalState>;

The main use case for this I've seen so far is knowing when a command is finished so an extension knows what state a terminal is in and can react accordingly (reuse terminal, close and open new, open new).

@jrieken
Copy link
Member

jrieken commented Nov 16, 2016

I see

@dbaeumer
Copy link
Member

What would be helpful as well if I could start a terminal and execute exactly one command and then terminate the terminal as well.

@Tyriar
Copy link
Member Author

Tyriar commented Nov 16, 2016

@dbaeumer is the reason for using the terminal over an output channel its support for colors? I've thought about the idea of making an integrated terminal instance read only to prevent unwanted interaction (which would probably be wanted for this).

@dbaeumer
Copy link
Member

It is color support. But I have also the request that a task should allow user input.

@Tyriar
Copy link
Member Author

Tyriar commented Nov 16, 2016

Created #15584 based on our discussion.

@Tyriar
Copy link
Member Author

Tyriar commented Mar 9, 2017

Closing in favor of #20676

@Tyriar Tyriar closed this as completed Mar 9, 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
api feature-request Request for new features or functionality terminal Integrated terminal issues
Projects
None yet
Development

No branches or pull requests

6 participants