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

Paste is annoyingly asynchronous wrt typeahead #15315

Open
KalleOlaviNiemitalo opened this issue May 9, 2023 · 5 comments
Open

Paste is annoyingly asynchronous wrt typeahead #15315

KalleOlaviNiemitalo opened this issue May 9, 2023 · 5 comments
Labels
Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Attention The core contributors need to come back around and look at this ASAP. Priority-3 A description (P3) Product-Terminal The new Windows Terminal.
Milestone

Comments

@KalleOlaviNiemitalo
Copy link

Windows Terminal version

1.17.1023

Windows build number

10.0.19045.2846

Other Software

git version 2.40.0.windows.1

Steps to reproduce

Run Git Bash in Windows Terminal, while other processes are causing CPU and paging load.

Copy some text to the Clipboard from another application. (For example, copy a Git commit ID.)

In Git Bash, type some characters, press Shift+Insert to paste, and type more characters. (For example, type git cherry-pick initially, and press Enter at the end.)

Expected Behavior

Pasted text should appear in the correct place between the first and second sequence of characters.

Actual Behavior

Pasted text occasionally appears after the second sequence of characters. As if Shift+Insert starts an asynchronous paste operation that completes at some later time and injects its result into the terminal input at that time.

@KalleOlaviNiemitalo KalleOlaviNiemitalo added Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels May 9, 2023
@KalleOlaviNiemitalo
Copy link
Author

KalleOlaviNiemitalo commented May 9, 2023

I think Windows Terminal should postpone the subsequent keyboard input events until the paste operation finishes. This should apply only to those events that become input to the session. It should not apply to events that the Terminal itself handles, like switching tabs or searching. Nor should it apply to reports that Terminal generates in response to requests from the session, rather than from user input.

If the clipboard owner hangs the paste operation altogether, then Windows Terminal could detect that after a timeout and display an information bar that lets the user cancel the paste by clicking an abort button or by pressing Ctrl+Break. If the user cancels the paste, Terminal should also discard all the postponed input of that session, to prevent it from running commands that don't include all the text originally intended by the user. (Not sure how this should behave if broadcast input mode #2634 has been started or stopped during the paste operation.)

@PankajBhojwani PankajBhojwani added Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Priority-3 A description (P3) labels May 10, 2023
@PankajBhojwani PankajBhojwani added this to the Terminal v1.19 milestone May 10, 2023
@PankajBhojwani
Copy link
Contributor

Thanks for filing this issue! Yes, this is definitely a bug. This behaviour might have improved from PR #15328 (probably not completely fixed though... so we'll leave this issue open)

@PankajBhojwani PankajBhojwani removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label May 10, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label May 10, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label May 15, 2023
@KalleOlaviNiemitalo
Copy link
Author

The "In-PR" label should be removed because #15360 no longer fixes this according to #15360 (comment).

@zadjii-msft zadjii-msft added Product-Terminal The new Windows Terminal. and removed In-PR This issue has a related PR labels May 24, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs-Tag-Fix Doesn't match tag requirements label May 24, 2023
microsoft-github-policy-service bot pushed a commit that referenced this issue Sep 21, 2023
The Win32 API is significantly faster than the WinRT one, in the order
of around 300-1000x depending on the CPU and CPU load.

This might slightly improve the situation around #15315, but I suspect
that it requires many more fixes. For instance, we don't really have a
single text input "queue" into which we write. Multiple routines that
`resume_background` just to `WriteFile` into the input pipe are thus
racing against each other, contributing to the laggy feeling.
I also fear that the modern Windows text stack might be inherently
RPC based too, producing worse lag with rising CPU load.

This might fix #14323

## Validation Steps Performed
* Paste text from Edge ✅
* Paste text from Notepad ✅
* Right click the address bar in Explorer, choose "Copy address",
  paste text into WT ✅

---------

Co-authored-by: Dustin L. Howett <duhowett@microsoft.com>
@zadjii-msft
Copy link
Member

Maybe this got better in 1.19/?

@zadjii-msft zadjii-msft modified the milestones: Terminal v1.19, Backlog Oct 4, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. labels Oct 4, 2023
@KalleOlaviNiemitalo
Copy link
Author

I haven't seen the symptom in 1.19 yet, but TerminalPage::_PasteFromClipboardHandler still does co_await winrt::resume_background(); before it uses the clipboard API and then calls eventArgs.HandleClipboardData(std::move(text)) on the background thread, so I don't believe #15360 can have fixed this completely.

@microsoft-github-policy-service microsoft-github-policy-service bot added Needs-Attention The core contributors need to come back around and look at this ASAP. and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something No-Recent-Activity This issue/PR is going stale and may be auto-closed without further activity. labels Oct 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-TerminalControl Issues pertaining to the terminal control (input, selection, keybindings, mouse interaction, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Attention The core contributors need to come back around and look at this ASAP. Priority-3 A description (P3) Product-Terminal The new Windows Terminal.
Projects
None yet
Development

No branches or pull requests

3 participants