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

fix(cli): Enforce a human delay in prompt to fix paste problem #23184

Merged
merged 11 commits into from
Apr 2, 2024

Conversation

mmastrac
Copy link
Contributor

@mmastrac mmastrac commented Apr 2, 2024

The permission prompt doesn't wait for quiescent input, so someone pasting a large text file into the console may end up losing the prompt. We enforce a minimum human delay and wait for a 100ms quiescent period before we write and accept prompt input to avoid this problem.

This does require adding a human delay in all prompt tests, but that's pretty straightforward. I rewrote the locked stdout/stderr test while I was in here.

if i > 5 {
panic!("Output happened before permission prompt too many times");
}
let context = TestContextBuilder::new().build();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rewrote this test to be more reliable -- no retries needed.

ext/ffi/turbocall.rs Outdated Show resolved Hide resolved
@mmastrac mmastrac merged commit 3b9fd1a into denoland:main Apr 2, 2024
17 checks passed
mmastrac added a commit that referenced this pull request Apr 4, 2024
satyarohith pushed a commit that referenced this pull request Apr 11, 2024
The permission prompt doesn't wait for quiescent input, so someone
pasting a large text file into the console may end up losing the prompt.
We enforce a minimum human delay and wait for a 100ms quiescent period
before we write and accept prompt input to avoid this problem.

This does require adding a human delay in all prompt tests, but that's
pretty straightforward. I rewrote the locked stdout/stderr test while I
was in here.
satyarohith pushed a commit that referenced this pull request Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants