Update crossterm to version 0.26.1

[WindSoilder]

WARN: do not merge this yet, it's a breaking change to nushell.

After investigate, the main changes from crossterm 0.24.0 to 0.26.1 includes the following:

1. remove crossterm::cursor::CursorShape and crossterm::cursor::SetCursorShape, using crossterm::cursor::SetCursorStyle instead.
2. supports kitty_keyboard_protocol, so crossterm::event::Event adds kind and state fields. In the case, I don't think we need to take care of state field, but need to be careful to kind field, when we release a key, crossterm will fire a Release event.

Followed by @sholderbach 's suggestion, I create a ReedlineRawEvent, which is a wrapper for crossterm::event::Event, ReedlineRawEvent will make sure that it won't be created by Release event. Maybe it's better to create reedline::KeyEvent, but I think filter the event at Event level rather than KeyEvent leval is easier, so I just leave it away for now :-)

Also add an example event_listener_kitty_proto.rs to play with kitty protocol.

Additional note about wezterm:

wezterm have an issue about Release event, it will pass Press event to crossterm. So we have to suggest user to turn off enable_kitty_keyboard option, luckly it's disabled by default. Here is the relative issue: wezterm/wezterm#3220

[fdncred]

Nice. I'd be happy to close #464 if this implements the stuff it was trying to.

[WindSoilder]

Oh, sorry I don't know #464 exists, I should check it first :-(

This pr is just a migration without adding any new features.

After taking a closer look at #464, I found it didn't handle KeyEventKind::Release, I guess it may generate 2 d if we just type d on windows sytem or some other terminals.

[fdncred]

Oh, sorry

No worries at all. I'm super pumped to see you work on this. Great work!

After taking a closer look at #464, I found it didn't handle KeyEventKind::Release, I guess it may generate 2 d if we just type d on windows system or some other terminals.

#464 was very incomplete. I'd go with your implementation but just add the things that were left out that are in 464 maybe?

[WindSoilder]

Sure, I can try it :-) Maybe the most tricky part it how to handle Paste Event

[fdncred]

We could also do it in 2 steps.

1. land something that gets us on the latest and works in nushell
2. land other enhancements like paste

[WindSoilder]

I'd prefer to do it in 2 steps, to keep our changeset smaller :-)

[WindSoilder]

Blocked by sharkdp/lscolors#58

[sholderbach]

Thanks for tackling this big challenge @WindSoilder!

That whole wezterm bug is really driving home what a mess the whole terminal "protocol" is on the edges. Props to the crossterm team for still making it work!

Sorry that I don't have too much time this week to review or red-team this PR.

Things I would check manually as they are not really directly tested on our side:

• entering non-ASCII characters both in uppercase and lowercase (we had to special-case that in the past for characters that use AltGr on European keyboards which also maps in some cases to Ctrl-Alt Modifier for altgr #139 ).
• paste behavior for single and multiline input.
• If modifier keys still work with special keys like Alt-arrow etc.

[fdncred]

From my testing, I haven't seen any problems. I've only tested on windows with Windows Terminal and with WezTerm with enable_kitty_keyboard = true.

[sholderbach]

Gave it some manual testing as well and seems to keep all expected functionality.

I would like to see this landed with some priority so we can finally get rid of the duplicated old crossterm version in our dependencies.

Great work @WindSoilder !

[fdncred]

I think there's 2 options to landing this now.

1. wait until sharkdp makes a new LS_COLORS release
2. include the lscolors crate pointing to the branch that has this fix, which landed a couple weeks ago. Only problem is, if there isn't a new release by the time we release in ~3weeks we'll have to roll back i think.

[WindSoilder]

I'd prefer 1st option, and created a relative issue to ask for a new version..

sharkdp/lscolors#63

[WindSoilder]

I think there's 2 options to landing this now.

Since we don't get reply yet, I'd propose 3rd option: copy-paste lscolors' crossterm relative code in the pr: nushell/nushell@e423dca

Then we can play with latest crossterm version. And we can adjust our dependency once lscolor is published.

[sholderbach]

Option 4: (I think would also need a version change) use nu_ansi_term instead of crossterm to interface with lscolors. It is lighterweight (should finish earlier in the compile graph) and has been seeing less changes in recent months.

addendum I think I tried this way back and ran into some issue but I think that should be solve (either the nuansi Deref weirdness or the fact that it was just ansiterm)

[fdncred]

I just noticed that lscolors has a nu-ansi-term feature. We should use that.

[WindSoilder]

Option 4: (I think would also need a version change)

@sholderbach Yeah, it's still need lscolor version change too....just create a pr to update it...
sharkdp/lscolors#64

Edit: we can't update nu-ansi-term to latest version in lscolors, because CI/CD check failed: https://github.com/sharkdp/lscolors/actions/runs/4676435880/jobs/8282744211, which requires rustc 1.58 version

[WindSoilder]

Hooray! lscolors released a new version with latest crossterm and nu_ansi_term version, I've adjusted nushell pr, so it's ready for now

[fdncred]

I vote to land this. Even if it doesn't enable all the features of crossterm 26.1, it gets us on that version where we can add those features later that were in my original PR for crossterm 25.

[sholderbach]

Thanks so much! Let's go

[fdncred]

Leaving this here in case we start seeing problems. Some people are having problems with crossterm + Windows. See the linked issues here. mikaelmello/inquire#132