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.

Sign up for GitHub

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

Jump to bottom

Restore the cursor shape when reedline exits #574

Merged
merged 1 commit into from Apr 24, 2023
Merged

Restore the cursor shape when reedline exits #574

merged 1 commit into from Apr 24, 2023

Conversation

nibon7
Copy link
Contributor

@nibon7 nibon7 commented Apr 22, 2023

Restore the cursor shape when reedline exits.
Tested on Linux (Alacritty 0.13.0) and Windows terminal.

@fdncred
Copy link
Collaborator

fdncred commented Apr 22, 2023 •
edited

Nice @nibon7, people have been asking for this. One question, does SetCursorStyle::DefaultUserShape set it to what the cursor shape used to be before launching reedline/nushell, or is it setting it to some default that could still be different than what was originally used?

Update: Answering my own question here. It looks like it sets it to blinking block.

SetCursorStyle::DefaultUserShape => f.write_str("\x1b[0 q"),

I'm wondering if this does what we want it to do, which is restore the users cursor to whatever it was prior to using reedline/nushell.

Ansi escape values for cursor.

CSI Ps SP q
          Set cursor style (DECSCUSR), VT520.
            Ps = 0  ⇒  blinking block.
            Ps = 1  ⇒  blinking block (default).
            Ps = 2  ⇒  steady block.
            Ps = 3  ⇒  blinking underline.
            Ps = 4  ⇒  steady underline.
            Ps = 5  ⇒  blinking bar, xterm.
            Ps = 6  ⇒  steady bar, xterm.

@fdncred
Copy link
Collaborator

fdncred commented Apr 22, 2023

This makes it sound like restoring the cursor to whatever it was originally isn't possible across all platforms crossterm-rs/crossterm#646

@nibon7
Copy link
Contributor Author

nibon7 commented Apr 23, 2023

This makes it sound like restoring the cursor to whatever it was originally isn't possible across all platforms crossterm-rs/crossterm#646

Maybe. I tested on windows and linux, and it seems ok.

  • Windows Terminal (block)
windows_block.mp4
  • Windows Terminal (underscore)
windows_underscore.mp4
  • Linux Alacritty (block)
linux_block.mp4
  • Linux Alacritty (underscore)
linux_underscore.mp4

@fdncred
Copy link
Collaborator

fdncred commented Apr 23, 2023 •
edited

I'm fine with moving ahead with it, even though it doesn't seem like it would work. Your examples seem to prove otherwise.

sholderbach
sholderbach reviewed Apr 23, 2023
View reviewed changes
src/engine.rs Outdated Show resolved Hide resolved
@sholderbach
Copy link
Member

Thanks!

@sholderbach sholderbach merged commit 86beb87 into nushell:main Apr 24, 2023
6 checks passed
@nibon7 nibon7 deleted the restore-cursor-shape branch April 25, 2023 00:19
@maxomatic458
Copy link
Contributor

is there a way to disable this behavior?
because im unable to use the block cursor in windows terminal now

@sholderbach
Copy link
Member

is there a way to disable this behavior?

Not yet/(not sure if nushell will unset the cursor_shape option when none is given in the config)

because im unable to use the block cursor in windows terminal now

Based on the settings on Windows terminal or inside nushell?

@maxomatic458
Copy link
Contributor

i set my cursor to block in windows terminal.
Is there a option in nushells config for the cursor?

@sholderbach
Copy link
Member

i set my cursor to block in windows terminal. Is there a option in nushells config for the cursor?

In the config we have the option here to allow you to disambiguate between the vi modes

https://github.com/nushell/nushell/blob/d45e9671d41fa7625de9d337704c8903de35ebbe/crates/nu-utils/src/sample_config/default_config.nu#L286-L290

@maxomatic458
Copy link
Contributor

That worked for me.
Thank you

@nibon7 nibon7 mentioned this pull request May 29, 2023
Merged
fdncred pushed a commit to nushell/nushell that referenced this pull request May 30, 2023
@nibon7
restore cursor shape when exits (#9314)
560c2e6 
# Description
This PR restores cursor shape when nushell exists.

Fixes #9243 

Related
[nushell/reedline#574](nushell/reedline#574)

# User-Facing Changes
<!-- List of all changes that impact the user experience here. This
helps us keep track of breaking changes. -->

* windows


https://github.com/nushell/nushell/assets/15247421/ede8d1c0-ecd1-40b0-87b0-6393c1a7160f


* linux


https://github.com/nushell/nushell/assets/15247421/b428f17e-05cb-45ad-aa5f-3a9753fd9176

* macos


https://github.com/nushell/nushell/assets/15247421/5170dabd-8b9f-4bad-a7a2-bdabfca45cca

# Tests + Formatting
<!--
Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- `cargo fmt --all -- --check` to check standard code formatting (`cargo
fmt --all` applies these changes)
- `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
clippy::needless_collect -A clippy::result_large_err` to check that
you're using the standard code style
- `cargo test --workspace` to check that all tests pass
- `cargo run -- crates/nu-std/tests/run.nu` to run the tests for the
standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->

# After Submitting
<!-- If your PR had any user-facing changes, update [the
documentation](https://github.com/nushell/nushell.github.io) after the
PR is merged, if necessary. This will help us keep the docs up to date.
-->
@mb21
Copy link

mb21 commented Sep 7, 2023

For reference, this seems to work on Apple Terminal on macOS 13.5.1 (block):

Screen.Recording.2023-09-07.at.09.50.09.mov

This was referenced Sep 7, 2023
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sholderbach sholderbach sholderbach left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@nibon7 @fdncred @sholderbach @maxomatic458 @mb21