Skip to content

Conversation

@KilianVounckx
Copy link
Contributor

Fixes this issue

Copy link
Collaborator

@Anton-4 Anton-4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution @KilianVounckx!
In the atomic bool docs I saw that " This type is only available on platforms that support atomic loads and stores of u8.", so I'd like to fall back to the std bool using #[cfg(target_has_atomic = "8")]. Given that this complicates things I'd also like to move the bulk of this complexity to a separate file.

What do you think? Feel free to take your time :)

@KilianVounckx
Copy link
Contributor Author

If I understand correctly, you want to fall back on a normal (non atomic) bool? Wouldn't this break multi-threading?

@Anton-4
Copy link
Collaborator

Anton-4 commented Jun 26, 2023

Yeah, I was thinking about sacrificing that as a trade-off.
Thinking about it more, I believe the functions in Tty.roc are out of scope for basic-cli, I'm going to start a discussion on zulip.

@Anton-4
Copy link
Collaborator

Anton-4 commented Jun 27, 2023

Based on the discussion on zulip, let's go with the proposed disable_raw_mode on panic, without inspecting the current state.

KilianVounckx and others added 4 commits June 27, 2023 17:42
With expect we would crash and lose the msg that was the cause of the panic. I thought about printing an error if `disable_raw_mode()` returned an `Err` but I think it would distract from the cause of the panic, so I chose to ignore the result.
@Anton-4 Anton-4 merged commit 524b2bd into roc-lang:main Jun 27, 2023
@Anton-4
Copy link
Collaborator

Anton-4 commented Jun 27, 2023

Thanks @KilianVounckx!

@KilianVounckx KilianVounckx deleted the panic-disable-raw-mode branch June 27, 2023 16:50
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.

Disable raw mode on panic

2 participants