Skip to content

feat(shortcuts)!: remove setRawMode#14342

Merged
bluwy merged 1 commit intomainfrom
no-raw-mode
Sep 21, 2023
Merged

feat(shortcuts)!: remove setRawMode#14342
bluwy merged 1 commit intomainfrom
no-raw-mode

Conversation

@bluwy
Copy link
Copy Markdown
Member

@bluwy bluwy commented Sep 10, 2023 •
edited
Loading

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
@bolt-new-by-stackblitz
Copy link
Copy Markdown

bolt-new-by-stackblitz bot commented Sep 10, 2023

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

@bluwy bluwy added this to the 5.0 milestone Sep 14, 2023
@silverwind silverwind mentioned this pull request Sep 14, 2023
Closed
5 tasks
@ArnaudBarre
Copy link
Copy Markdown
Member

ArnaudBarre commented Sep 16, 2023

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

@brc-dd brc-dd mentioned this pull request Sep 20, 2023
Merged
@bluwy
Copy link
Copy Markdown
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
@bluwy bluwy deleted the no-raw-mode branch September 21, 2023 09:11
@bluwy bluwy mentioned this pull request Dec 1, 2023
Merged
@WORMSS
Copy link
Copy Markdown
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

Reviewers

@ArnaudBarre ArnaudBarre ArnaudBarre approved these changes

Assignees

No one assigned

Labels

p3-significant High priority enhancement (priority)

Projects

Team Board
Archived in project

Milestone

5.0

Development

Successfully merging this pull request may close these issues.

suspend is not working in dev mode

3 participants

@bluwy @ArnaudBarre @WORMSS