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

API: onDidWriteTerminalData event #78502

Closed
Tyriar opened this issue Aug 5, 2019 · 5 comments
Closed

API: onDidWriteTerminalData event #78502

Tyriar opened this issue Aug 5, 2019 · 5 comments
Assignees
Labels
api api-proposal feature-request Request for new features or functionality terminal General terminal issues that don't fall under another label verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@Tyriar
Copy link
Member

Tyriar commented Aug 5, 2019

This has been in for several months and is used by remote extensions:

	export interface Terminal {
		/**
		 * Fires when the terminal's pty slave pseudo-device is written to. In other words, this
		 * provides access to the raw data stream from the process running within the terminal,
		 * including VT sequences.
		 */
		readonly onDidWriteData: Event<string>;
	}

I might tweak the wording to indicate it will fire for extension/pty terminals as well.

@Tyriar Tyriar added feature-request Request for new features or functionality api terminal General terminal issues that don't fall under another label labels Aug 5, 2019
@Tyriar Tyriar added this to the August 2019 milestone Aug 5, 2019
@Tyriar Tyriar self-assigned this Aug 5, 2019
@Tyriar
Copy link
Member Author

Tyriar commented Aug 5, 2019

Plan from meeting: Make it a global event and keep in proposed.

@roblourens
Copy link
Member

It has worked fine for me

@Tyriar Tyriar changed the title Stabilize Terminal.onDidWriteData API Make Terminal.onDidWriteData a global event Aug 5, 2019
@jrieken
Copy link
Member

jrieken commented Aug 6, 2019

Summary of the discussion during the API sync

  • the current proposal violated the "public object, global event rule", see: https://github.com/microsoft/vscode/wiki/Extension-API-guidelines#global-events
  • the private event approach was selected because there are strong performance concerns that forwarding events for all terminals is too expensive.
  • however, users like LS, need to listen to all terminals so that they can be replicated,
  • which means subscribing to all "private" events of all terminals is like a global events (and also rules out the possibility to only allow creators of terminals to listen)

Going forward

  • the API will change to be a global event
  • the API will likely stay proposed forever (which gives us more control)
  • the API needs a fast and smart implementation
  • we should gather telemetry data about how much data during a terminal session is usually produced (real world case)

@Tyriar
Copy link
Member Author

Tyriar commented Aug 22, 2019

Verifier: Try use the new API and make sure it works for terminals created before it was registered and after it was registered.

@jrieken jrieken added the verified Verification succeeded label Aug 27, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Sep 20, 2019
@Tyriar
Copy link
Member Author

Tyriar commented Nov 29, 2019

Update: This API is still proposed but we don't intent on promoting it to stable due to problems around performance. See #145234 for a more likely API to get stabilized.

@Tyriar Tyriar changed the title Make Terminal.onDidWriteData a global event API: onDidWriteTerminalData event Aug 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api api-proposal feature-request Request for new features or functionality terminal General terminal issues that don't fall under another label verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

3 participants