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

Reset state upon manual terminal closing #70

Merged
merged 8 commits into from
Oct 31, 2024

Conversation

Splines
Copy link
Member

@Splines Splines commented Oct 31, 2024

With this PR, we detect the event of a user manually closing a terminal that hosts the active Manim IPython session. Further behavior is described in the docstrings:

/**
* This event is fired when a terminal is closed manually by the user.
* In this case, we can only do our best to clean up since the terminal
* is probably not able to receive commands anymore. It's mostly for us
* to reset all states such that we're ready for the next command.
*
* When closing while previewing a scene, the terminal is closed, however
* we are not able to send a keyboard interrupt anymore. Therefore,
* ManimGL will take a few seconds to actually close its window. When
* the user employs our "Quit preview" command instead, the preview will
* be closed immediately.
*/

Some reset events might be fired multiple times in the end, but it's better to be safe than sorry here. Multiple calls to resetActiveShell() from different places of the code is not a problem.

While testing this, please try to force-quit the terminal manually in many different situations to make sure that no state is left dangling, e.g. some notifications still showing or subsequent commands not working anymore.

@Splines Splines self-assigned this Oct 31, 2024
@Splines Splines marked this pull request as ready for review October 31, 2024 19:56
@VladimirFokow
Copy link
Collaborator

VladimirFokow commented Oct 31, 2024

works for me
(tested several edge cases, didn't find problems)

@VladimirFokow VladimirFokow self-requested a review October 31, 2024 21:15
@Splines Splines merged commit f1ac0d8 into main Oct 31, 2024
@Splines Splines deleted the feature/detect-terminal-closing branch October 31, 2024 21:22
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.

2 participants