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

<C-u> to clear watch mode pattern prompt #11296

Closed
jeysal opened this issue Apr 14, 2021 · 12 comments · Fixed by #11358
Closed

<C-u> to clear watch mode pattern prompt #11296

jeysal opened this issue Apr 14, 2021 · 12 comments · Fixed by #11358

Comments

@jeysal
Copy link
Contributor

jeysal commented Apr 14, 2021

🚀 Feature Proposal

Currently, pressing Ctrl+u in a watch mode pattern prompt inserts an invisible character that also somewhat breaks searching because the files/tests searched for do not contain the character.
In most terminal environments, Ctrl+u clears the whole line. I think this should happen in the watch mode pattern prompt as well.

Motivation

Many users will expect Ctrl+u to behave as elsewhere in their terminal environment.
The workaround requires pressing backspace a lot to clear the line manually.

Pitch

Why does this feature belong in the Jest core platform?

Since jest-watchers Prompt is reused in third-party packages like jest-watch-typeahead, making the improvement here would benefit those as well as users of plain Jest watch mode prompts.

@SimenB
Copy link
Member

SimenB commented Apr 14, 2021

This is a Bash thing, but it probably works in most shell emulators (it does in zsh which I use at least).

If we add ctrl+u, we should also do others, like meta/ctrl+b and meta/ctrl+f etc. Feels like a somewhat slippery slope, but perhaps there's some library we can use which parses the input and returns a "command" of some sort that's easier to understand? Or even better would be some "cursor" library which, given a string and a position can parse terminal input and return a new string (if it modifies it) and cursor position. I've done zero research into whether such a thing already exists 🙂

@jeysal
Copy link
Contributor Author

jeysal commented Apr 14, 2021

This is a Bash thing, but it probably works in most shell emulators (it does in zsh which I use at least).

Yeah, but it's also supported in many non-shell places in "the terminal", like in Vim insert mode, when editing a tmux command line, ..., hence why I phrased it more generally as "terminal environments".

Not sure how much of a slippery slope it is, I don't think there are many common commands that would be meaningful in the pattern prompt, and especially none that are as ubiquitous across different applications as .

@SimenB
Copy link
Member

SimenB commented Apr 14, 2021

We should definitely have moving backwards and forwards (plus deleting words) if we start adding support for "commands". That might be it, though?

@jeysal
Copy link
Contributor Author

jeysal commented Apr 14, 2021

Yeah, but it's also supported in many non-shell places in "the terminal"

Oh, and perhaps notably is supported in the login prompt on Linux as well, before you even get a shell.

We should definitely have moving backwards and forwards (plus deleting words) if we start adding support for "commands". That might be it, though?

Yeah it seems moving forward and backward is about as ubiquitous. Would be nice if a library gives us everything for free - I've edited the description to say this should be checked - but I wouldn't mind introducing one without the other if it's easier since it's better than nothing.
Deleting words with also seems useful and almost as common (not supported in login prompt, where I guess it wouldn't be very helpful anyway, but just about anywhere else).

@parthsharma2
Copy link
Contributor

Hi @jeysal @SimenB could I work on this issue? I think I can implement the CTRL + U functionality.

Here is what I have in mind:

  • add CONTROL_U in the KEYS map in constants.ts
  • in the put method in the Prompt class whenever CONTROL_U is pressed set this._value = "", this._offset = -1, this._selection = null & call this._onChange()

@jeysal
Copy link
Contributor Author

jeysal commented Apr 24, 2021

@parthsharma2 sounds about right yeah :)

madhur1846 added a commit to madhur1846/jest that referenced this issue Apr 27, 2021
madhur1846 added a commit to madhur1846/jest that referenced this issue Apr 27, 2021
@parthsharma2
Copy link
Contributor

parthsharma2 commented Apr 28, 2021 via email

@jeysal
Copy link
Contributor Author

jeysal commented Apr 29, 2021

Hi @parthsharma2 and thanks a lot for doing this!
The changes from the commit look good to me, open a PR and we can let others review if they want and merge :)
The reason why you're not seeing the changes in manual testing is that we have jest-watch-typeahead plugins installed (see jest.config.js). These rely on the currently published version of jest-watcher and its Prompt. Once your changes are published, when jest-watcher is imported by jest-watch-typeahead the Prompt changes will also be available there. For now, you can see the changes in action by temporarily removing the relevant lines from out jest.config.js.

@parthsharma2
Copy link
Contributor

@jeysal @SimenB opened the PR #11358

@SimenB
Copy link
Member

SimenB commented May 10, 2021

@parthsharma2
Copy link
Contributor

@SimenB I could add support for ctrl + j & ctrl + k. Will create a PR soon

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants