Skip to content

HasUnsavedChanges reports false for never-saved states at position -1 (after trimming or branching), risking silent data loss on close #76

Description

@matt-edmondson

What's wrong

SaveBoundaryManager treats position -1 as "the clean original document" in two places, and neither holds once the stack has been trimmed or a boundary has been removed.

  1. HasUnsavedChanges (UndoRedo/Services/SaveBoundaryManager.cs:~21-24). When there are no boundaries it returns currentPosition >= 0, so -1 always counts as clean. But after MaxStackSize trimming, or after a branch removes the only boundary, -1 is no longer the original state.
  2. AdjustPositions (:~66) deletes a boundary when newPosition < 0. But -1 is a valid, reachable boundary position: MarkAsSaved on an empty stack creates one, and undoing after a trim reaches it. A save point that trimming shifts to exactly -1 is thrown away.

Failure scenarios (reproduced with a temporary MSTest)

A: trimming (MaxStackSize: 1).

  • Execute A (v=1), then Execute B (v=2). B trims A.
  • Undo gives v=1 pos=-1, and HasUnsavedChanges == false.
  • Edit A was never saved. An app that prompts "save before closing?" only when the flag is true loses it.

B: branching.

  • Execute A, then MarkAsSaved(), then Undo. HasUnsavedChanges is true, which is correct.
  • Execute B. The branch removes the boundary at 0.
  • Undo gives v=0 pos=-1, HasUnsavedChanges == false, with 0 boundaries.
  • The file on disk still holds A.

C: boundary dropped by trim (MaxStackSize: 2).

  • Execute A, MarkAsSaved("after A"), Execute B, Execute C.
  • The boundary should move to -1, but SaveBoundaries.Count == 0.
  • That save point can't be targeted by UndoToSaveBoundaryAsync. When other boundaries exist, position -1 wrongly reports unsaved.

Suggested fix

Track whether the initial state is clean explicitly instead of assuming it. For example:

  • Seed an implicit boundary at -1 on construction and on Clear(). Let CleanupInvalidBoundaries and AdjustPositions remove it like any other boundary, e.g. when trimming shifts it below -1, or when a branch invalidates it.
  • Change the removal condition in AdjustPositions to newPosition < -1.
  • Drop the "no boundaries means -1 is clean" fallback.

Acceptance: scenarios A and B report HasUnsavedChanges == true, and scenario C keeps one boundary at position -1.

Activity

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

Metadata

Metadata

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions