fix: put staged changes back when a rebase completes - #288
Open
dfaure-kdab wants to merge 1 commit into
Open
dfaure-kdab wants to merge 1 commit into
dfaure-kdab wants to merge 1 commit into
Conversation
git rebase --autostash replays its stash into the working tree only, so a staged modification came back unstaged on success, not just on abort. Only the abort path compensated, and drop, update and reword saved nothing at all, so every rebasing command quietly unstaged the user's work. Restore the saved patch on the RebaseOutcome::Completed arm and in every after_continue handler too, through one helper, and hand over what cannot be replayed rather than dropping it. Why it applies three-way against a rehearsed copy of the index rather than resetting is in Spec 014 and on the helper itself; the cost is a diff of the index per rebase, and a copy of it plus two applies per restore. Two recovery paths change with it: run_rebase_or_abort now aborts a rebase its own run_rebase left running instead of propagating past it, and reword keeps its state file when an abort failed, so loom abort still has it. The restore needs to ask whether a rebase is still on disk, so rebase_is_over names the check rebase_abort_then_cleanup was spelling out inline. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (36)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe change preserves staged changes across rebase completion, continuation, abortion, and rollback paths. It adds centralized three-way restoration, recovery parking, command-handler wiring, specifications, documentation, and regression tests. ChangesStaged preservation
Priority: ⬇️ Low Estimated code review effort: 4 (Complex) | ~60 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant UserCommand
participant run_rebase_or_abort
participant Git
participant Rollback
UserCommand->>run_rebase_or_abort: start rebase with saved staged patch
run_rebase_or_abort->>Git: run rebase with autostash
Git-->>run_rebase_or_abort: complete, stop, or fail
run_rebase_or_abort->>Rollback: restore or park staged patch
Rollback-->>UserCommand: return command result
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
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
1 participant
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.
git rebase --autostash replays its stash into the working tree only, so a
staged modification came back unstaged on success, not just on abort. Only
the abort path compensated, and drop, update and reword saved nothing at
all, so every rebasing command quietly unstaged the user's work.
Restore the saved patch on the RebaseOutcome::Completed arm and in every
after_continue handler too, through one helper, and hand over what cannot be
replayed rather than dropping it. Why it applies three-way against a
rehearsed copy of the index rather than resetting is in Spec 014 and on the
helper itself; the cost is a diff of the index per rebase, and a copy of it
plus two applies per restore.
Two recovery paths change with it: run_rebase_or_abort now aborts a rebase
its own run_rebase left running instead of propagating past it, and reword
keeps its state file when an abort failed, so loom abort still has it. The
restore needs to ask whether a rebase is still on disk, so rebase_is_over
names the check rebase_abort_then_cleanup was spelling out inline.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Summary by CodeRabbit
Bug Fixes
Documentation
Tests