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

feat(shortcuts)!: remove setRawMode #14342

Merged
merged 1 commit into from Sep 21, 2023
Merged

feat(shortcuts)!: remove setRawMode #14342

merged 1 commit into from Sep 21, 2023

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Sep 10, 2023

Description

Remove setRawMode for shortcuts to prevent the following caveats:

  1. Exit code is always 1. Why: fix(cli): exit 1 on ctrl+c #11563
  2. Suspending in background mode does not always work. Issue: suspend is not working in dev mode #11300
  3. Vite manually handles Ctrl-C and Ctrl-D, and blocks OS-specific shortcuts. Issue: suspend is not working in dev mode #11300
  4. Swallows user input. Can't enter-enter-enter to create a virtual separation in the CLI.

For the above reasons, it's best to not use setRawMode and let the OS and Node.js handle the inputs as usual. The downside is that every shortcuts needs to be followed with an Enter press to execute it (creates a new line to read the input).

Close #11300
Close #14319
Close #11418

Additional context


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

@bluwy bluwy added the p3-significant High priority enhancement (priority) label Sep 10, 2023
@stackblitz
Copy link

stackblitz bot commented Sep 10, 2023

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@ArnaudBarre
Copy link
Member

This also added benefit is that shortcut can be to chars long to choose a random letter if the first letter of a new aciton is already taken

@bluwy
Copy link
Member Author

bluwy commented Sep 21, 2023

Through some discussion, it's better to get this correct instead to avoid the potential issues. Vite's dev server is commonly used to pair with other processes and it would not be great if there's subtle issues with those.

I've made a poll (albeit only a small sample size) at https://twitter.com/bluwyoo/status/1704408350656782430 and https://elk.zone/m.webtoo.ls/@bluwy/111096467000284501. The rough average is that:

  • 57% of users are ok with this change
  • 22% of users are ok but slightly annoyed
  • 21% of users dislike this change

While it's still a significant portion of users who oppose this, the majority are still able to accept this. So we're going ahead with this change.

@bluwy bluwy merged commit 536631a into main Sep 21, 2023
12 checks passed
@bluwy bluwy deleted the no-raw-mode branch September 21, 2023 09:11
@WORMSS
Copy link
Contributor

WORMSS commented Feb 7, 2024

Is there any way to get the original behaviour back? It is highly annoying.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p3-significant High priority enhancement (priority)
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

suspend is not working in dev mode
3 participants