Fix Quit action being silently dropped (#892)#1614
Open
nikicat wants to merge 3 commits into
Open
Conversation
ContextManager::quit() emitted RioEvent::Quit, but application.rs only handled RioEvent::Exit, so the Quit binding and command-palette entry were silently dropped on Linux/Windows (macOS quits via a separate path). The two events were redundant: Quit was sent-but-never-handled, Exit handled-but-never-sent. Point the handler at RioEvent::Quit and remove the now-dead RioEvent::Exit variant. Fixes raphamorim#892 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The confirm dialog is an overlay drawn outside the terminal's damage tracking. While in ConfirmQuit the app runs a continuous redraw loop, but screen.render() discards any frame where no panel is dirty, dropping the overlay on idle frames. The dialog only appeared when the terminal happened to be dirty (recent output, cursor blink), so it showed intermittently. Mark the panel dirty after drawing the dialog so the frame is always presented. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> (cherry picked from commit 511f935)
Pressing N or Escape set the route back to Terminal but never requested a redraw, so the stale dialog overlay stayed on screen until the next event (any keypress) triggered a repaint. Request an overlay redraw so the terminal repaints immediately, matching the other dismiss paths in has_key_wait. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> (cherry picked from commit 19cf541)
Author
|
Pushed two follow-up commits that fix the quit-confirmation dialog (the overlay shown when
Happy to split these into a separate PR if you'd prefer to keep this one to just the event-dispatch fix. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The
Quitaction does nothing on Linux/Windows: keyboard binds and the command palette both callContextManager::quit(), which emitsRioEvent::Quit— butapplication.rsonly has a handler forRioEvent::Exit, so the event falls through and is silently dropped. (macOS is unaffected because it quits via theapplicationShouldTerminatepath.)The two events are redundant halves of the same intent:
RioEvent::Quit— sent (context/mod.rs:673) but never handledRioEvent::Exit— handled (application.rs:422, with theconfirm_before_quitlogic) but never sentThis consolidates them: the handler now matches
RioEvent::Quit(consistent withAction::Quit,PaletteAction::Quit,ContextManager::quit,Route::quit), and the deadRioEvent::Exitvariant is removed. The existing handler already respectsconfirm_before_quitand otherwise exits, so no behavior is lost.Fixes #892.
Test plan
cargo check -p rioterm -p rio-backendpasses with no new warnings[bindings]Quit entry now quits RioNote:
Action::Quitonly ships in the macOS default bindings, so Linux/Windows users still need a custom bind or the command palette to trigger it — but those paths now work. Adding a cross-platform default keybind is left as a separate decision.🤖 Generated with Claude Code