chore(deps): bump termwiz to 0.19.0

[xJonathanLEI]

Thanks to the swift action from Wez (not using @ here to avoid spamming his inbox), wez/wezterm#2694 has been merged and termwiz v0.19.0 has been released with the fix. This PR bumps the dependency version to v0.19.0 to take advantage of it.

This is part of the effort towards fully resolving #1021, and it should be much less disruptive compared to #1833.

[imsnif]

@tlinford - could you do me a favor and check this out on mac? Specifically to make sure all the default Ctrl keybindings work. I think it should be good but there was a breaking change and I want to make sure we won't cause trouble.

[xJonathanLEI]

Sure! Will test this one on my macbook. Not sure about what caused the CI failure though.

Update: I missed the @ part and thought you were asking me to test lol. Will still test on my side though.

[imsnif]

@a-kenji - any ideas why the nix build msrv is failing?

[a-kenji]

@imsnif,
The bump to 0.19.0 seems to be incompatible with our current advertised MSRV.
The termwiz release seems to need a more recent rust version. A minor version bump is probably sufficient.

[imsnif]

Thanks @a-kenji - can you help me understand how this mismatch affects us?

[tlinford]

Looks good to me - all keybindings in the default config seem fine.

[xJonathanLEI]

The current advertised MSRV is 1.59. I just tested with cargo msrv and it looks like:

• the actual MSRV on main right now is 1.59.0
• after this PR the actual MSRV is 1.60.0

I guess the question then becomes: do we want to bump MSRV? For your reference:

[xJonathanLEI]

For comparison, Helix has an MSRV of 1.61.0 on master. Personally I don't think MSRV matters too much...

[xJonathanLEI]

Optimistically bumping MSRV to 1.60.0 - this PR needs the bump after all.

[imsnif]

@xJonathanLEI - what do you mean by this PR needing the MSRV bump after all?

Honestly, my preference in these things is not to make changes unless they give us some benefit. If our actual MSRV is higher than what we advertise due to the dependencies we use... why does it matter? How does it affect us or our users? If there's an effect I'm not aware of - for sure. Let's bump it. But otherwise I'd opt to keep things as they are and remove this CI test.

[xJonathanLEI]

@imsnif MSRV indicates the minimum Rust compiler version that our code can be compiled with. It makes zero difference to users who always use the latest Rust tool chain version.

It only matters for users who are forced to use an old version of Rust:

• those who don't have permission to install stuff
• another project depending on us that maintains a lower MSRV (we are that "another project" right now)

This is when we changed our MSRV last time: #1177

This is where we "advertise" our MSRV:

zellij/Cargo.toml

Line 11 in 5975af6

rust-version = "1.59"

The CI is there to check the code can actually be compiled with the declared minimum version. If it fails, it means our advertised MSRV needs a bump to match the actual MSRV.

[imsnif]

The CI is there to check the code can actually be compiled with the declared minimum version. If it fails, it means our advertised MSRV needs a bump to match the actual MSRV.

I totally understand and am aware of MSRV - but what I don't understand is this part.

Why? Look how much time we're spending on this. If a user tries to compile with our MSRV version, they will get an error because of the MSRV in our dependencies. Same error they would get if we bump our own. What does it matter what we advertise? Why do we need to waste CI cycles on this? Is there something I'm missing?

[xJonathanLEI]

Well the CI is there to make sure we're following the standard, where it requires the rust-version field to be the MSRV. This helps save user time if they attempt to compile with an older version: cargo will just exit directly without trying to build it.

So, if the advertised version here is lower than the actual MSRV, like we're having here before the second commit, users of these Rust versions in between will have to compile and encounter an error to find out - not a big deal IMO. But this is technically the benefit of advertising the accurate MSRV (asserted by the CI) now that you asked :)

In fact, the benefit is so marginal that many projects I came across don't take it seriously. See here Helix is reporting a MSRV of 1.59 where it actually is 1.61.

[imsnif]

Thanks for the explanation @xJonathanLEI ! I'm good with merging this as is with the new version because we've already done the work. But this is honestly not something I want in our CI because I think - as you say - the benefit is far too marginal and it makes it harder to merge features and bug fixes. I'll remove it in another PR.

Anything else we need before merging? IMO we're good.

[xJonathanLEI]

I think we're good! I've been using this dependency bump for a while and didn't encounter any issue.

Additional context for the MSRV topic: helix-editor/helix#3913