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

Move to clearer reedline keyboard enhancement API #11045

Merged
merged 2 commits into from Nov 14, 2023

Conversation

sholderbach
Copy link
Member

Go from the ill-defined enable/disable pairs to .use_... builders
This alleviates unclear properties when the underlying enhancements are
enabled. Now they are enabed when entering Reedline::read_line and
disabled when exiting that.

Furthermore allow setting $env.config.use_kitty_protocol to have an
effect when toggling during runtime. Previously it was only enabled when
receiving a value from config.nu. I kept the warning code there to not
pollute the log. We could move it into the REPL-loop if desired

Not sure if we should actively block the enabling of bracketed_paste
on Windows. Need to test what happens if it just doesn't do anything we
could remove the cfg! switch. At least for WSL2 Windows Terminal
already supports bracketed paste. target_os = windows is a bad
predictor for conhost.exe.

Depends on nushell/reedline#659
(pointing to personal fork)

Closes #10982
Supersedes #10998

Go from the ill-defined `enable/disable` pairs to `.use_...` builders
This alleviates unclear properties when the underlying enhancements are
enabled. Now they are enabed when entering `Reedline::read_line` and
disabled when exiting that.

Furthermore allow setting `$env.config.use_kitty_protocol` to have an
effect when toggling during runtime. Previously it was only enabled when
receiving a value from `config.nu`. I kept the warning code there to not
pollute the log. We could move it into the REPL-loop if desired

Not sure if we should actively block the enabling of `bracketed_paste`
on Windows. Need to test what happens if it just doesn't do anything we
could remove the `cfg!` switch. At least for WSL2 Windows Terminal
already supports bracketed paste. `target_os = windows` is a bad
predictor for `conhost.exe`.

Depends on nushell/reedline#659
(pointing to personal fork)

Closes nushell#10982
Supersedes nushell#10998
@LevitatingBusinessMan
Copy link

LevitatingBusinessMan commented Nov 14, 2023

Not sure if we should actively block the enabling of bracketed_paste
on Windows. Need to test what happens if it just doesn't do anything we
could remove the cfg! switch. At least for WSL2 Windows Terminal
already supports bracketed paste. target_os = windows is a bad
predictor for conhost.exe.

As discussed on #10998, bracketed paste really shouldn't be disabled on Windows. Bracketed paste either works in terminals that support it or should be ignored on others. That is not related to the OS at all.

Also did you mean to commit your changes to Cargo.toml and Cargo.lock?

Other than that I think it looks good. Let's make sure it is tested properly.

@sholderbach
Copy link
Member Author

Agree that we should not make a full platform assumption and in the best case only dispatch based on the terminal emulator we are facing. Sadly crossterm itself currently makes effectively the same assumption that Windows doesn't really support all the ANSI escape codes.

Removing the cfg! and launching from both Powershell/cmd in Windows Terminal and the old console host did not show me bracketed paste behavior (we could work towards aligning the behavior on guessed paste with bracketed paste, but that is a separate challenge to tackle in reedline).

When trying to trigger the situation where crossterm doesn't make the assumption it lives in old Windows land, I ran into the issue described in #10808

@sholderbach sholderbach merged commit 1b3092a into nushell:main Nov 14, 2023
20 checks passed
hardfau1t pushed a commit to hardfau1t/nushell that referenced this pull request Dec 14, 2023
Go from the ill-defined `enable/disable` pairs to `.use_...` builders
This alleviates unclear properties when the underlying enhancements are
enabled. Now they are enabed when entering `Reedline::read_line` and
disabled when exiting that.

Furthermore allow setting `$env.config.use_kitty_protocol` to have an
effect when toggling during runtime. Previously it was only enabled when
receiving a value from `config.nu`. I kept the warning code there to not
pollute the log. We could move it into the REPL-loop if desired

Not sure if we should actively block the enabling of `bracketed_paste`
on Windows. Need to test what happens if it just doesn't do anything we
could remove the `cfg!` switch. At least for WSL2 Windows Terminal
already supports bracketed paste. `target_os = windows` is a bad
predictor for `conhost.exe`.

Depends on nushell/reedline#659
(pointing to personal fork)

Closes nushell#10982
Supersedes nushell#10998
dmatos2012 pushed a commit to dmatos2012/nushell that referenced this pull request Feb 20, 2024
Go from the ill-defined `enable/disable` pairs to `.use_...` builders
This alleviates unclear properties when the underlying enhancements are
enabled. Now they are enabed when entering `Reedline::read_line` and
disabled when exiting that.

Furthermore allow setting `$env.config.use_kitty_protocol` to have an
effect when toggling during runtime. Previously it was only enabled when
receiving a value from `config.nu`. I kept the warning code there to not
pollute the log. We could move it into the REPL-loop if desired

Not sure if we should actively block the enabling of `bracketed_paste`
on Windows. Need to test what happens if it just doesn't do anything we
could remove the `cfg!` switch. At least for WSL2 Windows Terminal
already supports bracketed paste. `target_os = windows` is a bad
predictor for `conhost.exe`.

Depends on nushell/reedline#659
(pointing to personal fork)

Closes nushell#10982
Supersedes nushell#10998
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.

Pasting with bracketed_paste creates escape codes that affect some programs
2 participants