Skip to content

Persist only the checkbox selection of uncommitted changes - #15198

Open
ameyypawar wants to merge 1 commit into
gitbutlerapp:masterfrom
ameyypawar:fix/10892-persist-selection-only
Open

Persist only the checkbox selection of uncommitted changes#15198
ameyypawar wants to merge 1 commit into
gitbutlerapp:masterfrom
ameyypawar:fix/10892-persist-selection-only

Conversation

@ameyypawar

Copy link
Copy Markdown
Contributor

🧠 Changes

uncommittedSlice is now persisted with treeChanges and hunkAssignments blacklisted, leaving only hunkSelection — the checkbox state — to survive a restart.

The blacklist alone would not have been enough. redux-persist applies it when writing, but getStoredState reads back every key on disk and autoMergeLevel1 merges all of them, so anything an earlier build had already written would still rehydrate once after upgrading. persistConfigFor() pairs the blacklist with a state reconciler that drops those keys on the way in as well.

☕️ Reasoning

injectPersistedSlice() persisted whole slices, and the uncommitted slice holds three things:

treeChanges      // the file list
hunkAssignments  // assignments
hunkSelection    // the checkbox state

The first two are refetched from the worktree on startup, so a persisted copy is only ever the previous session's, rendered for the frames before the fresh query replaces it. That is the flash in the report, and it fits the observation that deleting the cache directory makes it go away.

The blacklist lives next to the type it has to match, as const satisfies readonly (keyof UncommittedState)[], so a name that drifts from the slice shape fails the type check rather than silently re-persisting a key.

Three tests in persistBlacklist.test.ts drive the real slice through a real persist cycle under the config the app builds, with only storage swapped for a recording adapter. Two of them fail if persistConfigFor() is reverted to returning { key, storage }:

  • only the checkbox state is written — all three keys reach storage
  • state left by an earlier build is not rehydratedgone.txt is still there after rehydrate

The third asserts hunkSelection is not blacklisted. That one is a guard against the opposite mistake rather than a regression test, and passes either way.

One consequence worth stating plainly: a partial line selection inside a hunk comes back as the whole hunk checked after a restart, because updateLines() no longer has the previous assignment to diff against. If the file also changed while the app was closed, the reconciled assignment keeps its id but covers different lines, so a restored selection can be wider than what was originally chosen. Whole-hunk and whole-file checkboxes restore exactly.

Keeping hunkAssignments persisted would avoid that, at the cost of still rehydrating state this issue calls out as not being checkbox selection. I went with the narrower persistence; happy to swap to blacklisting only treeChanges if you would rather keep line-level precision across restarts.

🎫 Affected issues

Fixes: #10892

The uncommitted slice holds tree changes and hunk assignments alongside the
hunk selection, and all three were persisted. Both of the former are refetched
from the worktree on startup, so the persisted copy is only ever the previous
session's, rendered for the frames before the fresh query replaces it.

`blacklist` keeps them out of storage, and a state reconciler drops them on the
way in as well, so state written by an earlier build does not rehydrate once
more after upgrading.
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.

persistence of unassigned changes that survives application restarts

1 participant