Skip to content

Window-local Space saves: follow-ups from the #741 final review #1674

Description

@fluck-boss

From the merge review on #741 (none blocked the merge; finding 1 is the one to take first):

  1. saveOwner is a non-reactive read captured at composition (WorkspaceButton.kt:99). SplitViewStateRegistry.getState reads a MutableStateFlow outside Compose snapshot state, and registration happens in a LaunchedEffect after first composition, so the dialog's rebind fence can silently drop a save for a live window and log "deregistered" for one that never registered. Resolve the owner at press time inside onSave, fence against that local, key the latch on windowId alone, and make the miss log distinguish never-registered from deregistered.
  2. The call-site wiring is the untested half, and every defect in this series lived there. Extracting the menu effect's save wiring into an internal function would let ordinary unit tests pin: snapshot built from the invoking window's own id, a second event coalescing into exactly one re-run on the latest layout, re-run dropped after deregistration, dialog ignoring rather than replaying.
  3. SaveInFlightLatch.press()/begin() is a two-step that only call-site discipline keeps safe; fold into tryStart() or document that begin() must be reached synchronously, and note the latch is main-thread-only by design.
  4. An ignored overlapping named save is silent - a debug log on the ignored press closes it at no cost.

Smalls carried from the same review: the eight-line queued-re-run block duplicated between onSaved and onFailed; WindowSpaceSaveIdentityTest using fully-qualified imports its sibling imports; the named save's success-toast asymmetry.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions