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 API: debounce onData()? #48513

Closed
bpasero opened this issue Apr 24, 2018 · 4 comments · Fixed by #82189
Closed

Terminal API: debounce onData()? #48513

bpasero opened this issue Apr 24, 2018 · 4 comments · Fixed by #82189
Assignees
Labels
debt Code quality issues help wanted Issues identified as good community contribution opportunities perf terminal Integrated terminal issues verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@bpasero
Copy link
Member

bpasero commented Apr 24, 2018

Refs: #48434

It looks like every single character I type produces a onData event in the terminal. Should this maybe be debounced? What is the scenario for having this API that it needs to fire on each keystroke?

@Tyriar
Copy link
Member

Tyriar commented Apr 24, 2018

I was actually introducing this at a lower level than this, but it would probably not be enabled for standard vscode terminals microsoft/node-pty#189, it may actually slow things down unless things are going over the wire.

Some debouncing would be a good idea between the renderer and extension host.

@Tyriar Tyriar added bug Issue identified by VS Code Team member as probable bug terminal Integrated terminal issues labels Apr 24, 2018
@Tyriar Tyriar added this to the May 2018 milestone Apr 24, 2018
@Tyriar Tyriar modified the milestones: May 2018, Backlog May 25, 2018
@Tyriar Tyriar added the help wanted Issues identified as good community contribution opportunities label May 25, 2018
@Tyriar Tyriar added the perf label Jun 22, 2018
@Tyriar Tyriar modified the milestones: Backlog, October 2019 Oct 8, 2019
Tyriar added a commit that referenced this issue Oct 22, 2019
Tyriar added a commit that referenced this issue Oct 23, 2019
eamodio pushed a commit that referenced this issue Oct 24, 2019
Fixes #48513 - buffers terminal onData events
@bpasero bpasero added debt Code quality issues and removed bug Issue identified by VS Code Team member as probable bug labels Oct 29, 2019
@eamodio eamodio added the verification-needed Verification of issue is requested label Oct 29, 2019
@JacksonKearl JacksonKearl added the verified Verification succeeded label Oct 29, 2019
@JacksonKearl
Copy link
Contributor

Is this user-facing? How can I verify?

@JacksonKearl JacksonKearl added verification-steps-needed Steps to verify are needed for verification and removed verified Verification succeeded labels Oct 30, 2019
@Tyriar
Copy link
Member

Tyriar commented Oct 30, 2019

@eamodio can you put some steps in please?

@eamodio
Copy link
Contributor

eamodio commented Oct 30, 2019

To test this you need to create an extension:

  1. Run npx -p yo -p generator-code -c 'yo code --extensionType=command-ts' to scaffold an extension
  2. Add "enableProposedApi": true to the package.json
  3. Change "activationEvents" in the package.json to "activationEvents": ["*"]
  4. Replace the activate function in extension.ts with the following
export function activate(context: vscode.ExtensionContext) {
    context.subscriptions.push(
        (vscode.window as any).onDidWriteTerminalData(
            (e: { readonly terminal: vscode.Terminal; readonly data: string }) => {
                console.log(`${e.terminal.name}\n${e.data}`);
            }
        )
    );
}
  1. Run the extension and switch to the Debug Console
  2. In the newly launched vscode extension host window, open a terminal and run an ls command or a command that will output a bunch of data
  3. Back in the original vscode window, in the Debug Console ensure you see the output from the terminal with multiple lines for each event (prior to these changes it would have only be 1 line per event at most)

@Tyriar Tyriar removed the verification-steps-needed Steps to verify are needed for verification label Oct 30, 2019
@JacksonKearl JacksonKearl added the verified Verification succeeded label Oct 30, 2019
@vscodebot vscodebot bot locked and limited conversation to collaborators Dec 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues help wanted Issues identified as good community contribution opportunities perf terminal Integrated terminal issues verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants