Skip to content

[Feature]: Ask to save unsaved changes when RustGIS is quit on macOS (Cmd+Q, Dock) #966

Open

Description

@Mavrick-F

Before you start

What problem does this solve?

#945 (for #906) asks Save changes to…? before the desktop app discards unsaved changes. On macOS, Quit RustGIS (⌘Q) and the Dock's Quit skip that question. AppKit terminates the app without a window close request, and eframe goes straight to its exiting handler, which notes: "On Mac, Cmd-Q we get here and then run_app_on_demand doesn't return". saveprompt::close_requested never sees the quit.

#945 limits the damage. An exit with unsaved changes the user never chose to discard keeps a last recovery copy, and the next start offers it back ("RustGIS was quit last time without saving your changes."). That still isn't the prompt Windows and Linux get. With autosave off, the work is lost.

Proposed solution

Route macOS application termination into the same Save / Don't Save / Cancel flow:

  • Answer applicationShouldTerminate: with NSTerminateCancel (or NSTerminateLater) while the project has unsaved changes. Raise the save prompt as saveprompt::close_requested does today.
  • Once the user chooses Save (and it succeeds) or Don't Save, terminate. With NSTerminateLater, that means replyToApplicationShouldTerminate:. Cancel keeps the app running.
  • Keep it inside apps/rustgis/src/macos.rs, beside the existing title-bar integration. Use the objc2 crates already in apps/rustgis/Cargo.toml, so no new dependency.

The blocker is unsafe. objc2 can only do this with unsafe code:

  • Delegate: defining an application delegate (or subclassing the one winit installs) uses define_class!, which needs unsafe impl.
  • Menu re-routing: pointing the Quit menu item at performClose: instead would use NSMenuItem::setAction, which objc2-app-kit 0.3 marks unsafe. It also wouldn't cover the Dock's Quit, which sends terminate: directly.

The workspace sets unsafe_code = "forbid". This needs a maintainer decision:

  1. Allow a narrow exception: a single #[allow(unsafe_code)] module in apps/rustgis (an app, not a library crate), reviewed and tested on macOS (ci:macos).
  2. Keep the recovery-copy behaviour from feat(ui): ask to save unsaved changes before quitting or replacing the project #945 and close this issue.
  3. Wait for winit to forward termination as a close request. Newer winit releases may expose application termination to the app, without any unsafe here.

A pull request would add a macOS-only test where possible. At minimum it would need a manual check on a Mac: ⌘Q, Dock Quit, and logout with unsaved changes, under each of the three answers.

Estimated scope

Small: a contained change, but it needs a macOS machine to verify and a decision on unsafe.

Alternatives considered

  • Hot exit (the recovery copy from feat(ui): ask to save unsaved changes before quitting or replacing the project #945): what macOS does now. It keeps the work, but it doesn't ask, and it does nothing with autosave off.
  • Replace winit's default app menu with one built without the terminate: Quit item, handling ⌘Q as an ordinary shortcut that runs ui.quit. That avoids unsafe for the menu, but removes the standard app menu, and the Dock's Quit still bypasses the prompt.

Raised in review of #945 (CodeRabbit).

Activity

  1. self-assigned this
    on Oct 26, 2026
  2. Mavrick-F commented on Oct 26, 2026

    @Mavrick-F
    MemberAuthor

    Looked into this upstream. The fix likely doesn't need unsafe in RustGIS after all.

    Cause. winit 0.30.13's macOS application delegate implements applicationWillTerminate: but not applicationShouldTerminate:. AppKit therefore terminates at once on terminate:. That covers the default menu's Quit (⌘Q), the Dock's Quit, logout and shutdown. No close request reaches eframe. eframe's own confirm_exit example has the same gap: emilk/egui#7115.

    Upstream routes:

    The local route this issue first described works on 0.30 only by adding the method to winit's delegate class at runtime (class_addMethod). That's unsafe and patches another crate's class, so I'd hold it back unless both routes above stall.

    Proposed plan: wait for whichever lands first. Then either enable the winit feature or take the eframe upgrade, and verify ⌘Q, Dock Quit and logout on a Mac. Until then, #945's recovery copy keeps the work when autosave is on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions