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

Fixed error detection in move_cursor_at_leftmost #434

Merged
merged 1 commit into from
Sep 2, 2020

Conversation

Lucretiel
Copy link
Contributor

@Lucretiel Lucretiel commented Sep 2, 2020

Code 87 emits an ErrorKind::InvalidInput, not ErrorKind::Other. Fixed the detection here by removing the ErrorKind detection entirely and using a winapi named constant. Fixes #433. See https://github.com/rust-lang/rust/blob/master/library/std/src/sys/windows/mod.rs#L64 for details.

@Lucretiel
Copy link
Contributor Author

Yup, looks like ERROR_INVALID_PARAMETER was moved to InvalidInput a couple months ago rust-lang/rust@b71a3e1

@gwenn
Copy link
Collaborator

gwenn commented Sep 2, 2020

Thanks.
And do you know which cursor position is invalid ? X, Y or both ?

@gwenn gwenn merged commit 31ce98c into kkawakam:master Sep 2, 2020
@Lucretiel
Copy link
Contributor Author

No idea; I'm not familiar enough with the relevant Windows API. I also don't know why in practice this error appears on Windows Terminal and Alacritty but not cmd.exe

@sophiajt
Copy link

Just confirming that this fixes the issue in Nushell. We'll keep testing, but aren't able to repro the failures we saw before.

@gwenn - would you be able to roll out a new rustyline release with this fix? That would let us move to it for the next Nushell release as well.

@Lucretiel
Copy link
Contributor Author

Unfortunately it seems like other people were having trouble even reproducing the issue in nushell, which surprised me. It's still under investigation.

@sophiajt
Copy link

@Lucretiel we managed to reproduce it this morning, luckily. Looks like this PR addresses the cases we could find.

@gwenn
Copy link
Collaborator

gwenn commented Sep 11, 2020

Thanks you all for your help.
I will try to release a new version this week-end.

gwenn added a commit that referenced this pull request Sep 13, 2020
Fixed error detection in move_cursor_at_leftmost
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.

Windows still panics in some cases
3 participants