Hard restore with X asks before dropping staged pins

X dropped staged pins without a question, including the selected
submodule's own pin, which the loss check didn't count. A staged pin is
a decision, and X sits one Shift away from x, so dropped staged pins now
count as a loss and make X ask (D31, amends D30).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-24 23:26:32 +02:00
co-authored by Claude Opus 5.5
parent 72500353cd
commit b205576759
9 changed files with 84 additions and 24 deletions
+3 -3
View File
@@ -117,9 +117,9 @@ something would be lost, you are asked first:
submodule with a staged pin, `x` first asks whether to restore to the staged submodule with a staged pin, `x` first asks whether to restore to the staged
pin (`s`) or the committed one (`c`, which unstages it). pin (`s`) or the committed one (`c`, which unstages it).
`X` is a hard restore: it goes to the committed pins without asking which `X` restores to the committed pins without asking which one, and also
one, and also drops the staged pins of submodules below the selection, drops the staged pins of submodules below the selection, which `x` keeps
which `x` keeps where it can. It still asks if work would be lost. where it can. It asks if work or a staged pin would be lost.
### Switch a branch with `b` ### Switch a branch with `b`
+4 -3
View File
@@ -87,9 +87,10 @@ own, and for nested repositories that are on their pin. A repository that
moves loses its staged pins either way (see above); then `x` → `c` and `X` moves loses its staged pins either way (see above); then `x` → `c` and `X`
end the same, but `c` asks first. end the same, but `c` asks first.
The dropped staged pins are counted in the loss (`2 staged pins`), but The dropped staged pins are counted in the loss (`2 staged pins`), the
alone they don't make restore ask: pressing `X` is the choice to drop them. selected submodule's own staged pin included, and like any loss they make
If something else would be lost, the question lists them with it. The restore ask (D31): `Restore libs/staged to the committed pins? Discards 1
staged pin.` The
commits of a dropped pin stay in the submodule; if HEAD leaves them and no commits of a dropped pin stay in the submodule; if HEAD leaves them and no
ref reaches them, they count as `commits on no branch`, as always. ref reaches them, they count as `commits on no branch`, as always.
+15
View File
@@ -593,3 +593,18 @@ staged and committed pin is still useful there.
the superproject. `restoreRepo` and `lossOf` take whether to keep staged the superproject. `restoreRepo` and `lossOf` take whether to keep staged
pins; switch always keeps them. Without a committed pin (an added pins; switch always keeps them. Without a committed pin (an added
submodule), `X` uses the index pin. submodule), `X` uses the index pin.
## D31 — Hard restore asks before dropping staged pins
**Context.** D30 let `X` drop staged pins without a question: pressing `X`
was the choice. In a usability walkthrough (see
[notes/usability-plan.md](../notes/usability-plan.md)) `X` on `libs/staged`
unstaged its pin silently. `X` is one Shift away from `x`, and users new to
submodules don't know where to find the dropped commit again.
**Decision.** Amends D30. Dropped staged pins are a loss like any other and
make `X` ask: `Restore libs/staged to the committed pins? Discards 1 staged
pin.` `RestoreLoss` with `RestoreHard` now also counts the selected
submodule's own staged pin (in its parent's index), which `lossOf` doesn't
see. The `c` answer of `x` stays silent: the user picked it in a question.
**Consequences.** `X` asks more often, but only where it changes what is
recorded. Without staged pins or other losses it still runs at once.
+1 -1
View File
@@ -150,7 +150,7 @@ commit hashes. It doesn't answer the questions users actually have:
confirmation only if something would be lost. See [actions.md](actions.md#restore). confirmation only if something would be lost. See [actions.md](actions.md#restore).
`X` is a hard restore: it goes to the committed pins at every depth, `X` is a hard restore: it goes to the committed pins at every depth,
without the pin question, and drops all staged pins below the selection without the pin question, and drops all staged pins below the selection
too. Dropped staged pins alone don't make it ask. See too. Dropped staged pins count as a loss, so it asks first. See
[actions.md](actions.md#hard-restore). [actions.md](actions.md#hard-restore).
- **F57** `b` opens a popup in the middle of the screen with a search line - **F57** `b` opens a popup in the middle of the screen with a search line
+3 -2
View File
@@ -363,8 +363,9 @@ the term*.
`y` restores, any other key cancels; the keys are not passed on while a `y` restores, any other key cancels; the keys are not passed on while a
question is open. After `c` the name reads `src/app to the committed pin`. question is open. After `c` the name reads `src/app to the committed pin`.
`X` (hard restore) never asks for the pin; its name reads `src/app to the `X` (restore committed pins) never asks for the pin; its name reads
committed pins`, and dropped staged pins alone don't make it ask. Without a loss there is no question. Success flashes `src/app to the committed pins`. Staged pins it drops count as a loss
(`Discards 1 staged pin.`), so it asks (D31). Without a loss there is no question. Success flashes
`restored src/app` in the footer. For the header the name is `the `restored src/app` in the footer. For the header the name is `the
superproject`. This line takes precedence over the filter line and the superproject`. This line takes precedence over the filter line and the
problems-only notice. problems-only notice.
+22 -3
View File
@@ -35,10 +35,29 @@ func (p *Provider) RestoreLoss(ctx context.Context, path string, to submodule.Re
if err != nil { if err != nil {
return submodule.Loss{}, err return submodule.Loss{}, err
} }
if dir == "" { // not initialized: nothing on disk to lose var loss submodule.Loss
return submodule.Loss{}, nil if dir != "" { // not initialized: nothing on disk to lose
loss = p.lossOf(ctx, dir, target, to.KeepsStaged())
} }
return p.lossOf(ctx, dir, target, to.KeepsStaged()), nil if to == submodule.RestoreHard && path != "" && p.pinStaged(ctx, path, target) {
// a hard restore unstages the submodule's own pin too
loss.StagedPins++
loss.Modules = max(loss.Modules, 1)
}
return loss, nil
}
// pinStaged reports whether the pin of path in its parent's index differs
// from committed, the pin a hard restore goes to.
func (p *Provider) pinStaged(ctx context.Context, path, committed string) bool {
p.mu.RLock()
in, ok := p.mods[path]
p.mu.RUnlock()
if !ok {
return false
}
staged, err := pinOf(ctx, in, submodule.RestoreStaged)
return err == nil && staged != committed
} }
// Restore makes path ("" is the superproject) and all submodules below it // Restore makes path ("" is the superproject) and all submodules below it
+23
View File
@@ -364,6 +364,29 @@ func TestRestoreHardOfASubmoduleUnstagesItsPin(t *testing.T) {
} }
} }
func TestRestoreLossOfAHardRestoreCountsTheOwnStagedPin(t *testing.T) {
p, _ := ownFixture(t)
committed, err := p.RestoreLoss(context.Background(), "libs/staged", submodule.RestoreCommitted)
if err != nil {
t.Fatal(err)
}
hard, err := p.RestoreLoss(context.Background(), "libs/staged", submodule.RestoreHard)
if err != nil {
t.Fatal(err)
}
if hard.StagedPins != committed.StagedPins+1 || hard.Modules == 0 {
t.Errorf("hard loss %+v, want the own staged pin on top of %+v", hard, committed)
}
// without a staged pin, a hard restore unstages nothing of its own
clean, err := p.RestoreLoss(context.Background(), "src/ui", submodule.RestoreHard)
if err != nil {
t.Fatal(err)
}
if clean.StagedPins != 0 {
t.Errorf("loss of src/ui %+v, want no staged pin", clean)
}
}
func TestRestoreLossCountsStagedPinsOfAMovingRepository(t *testing.T) { func TestRestoreLossCountsStagedPinsOfAMovingRepository(t *testing.T) {
p, root := ownFixture(t) p, root := ownFixture(t)
super := filepath.Join(root, "super") super := filepath.Join(root, "super")
+1 -7
View File
@@ -135,13 +135,7 @@ func (m Model) lossChecked(msg lossMsg) (tea.Model, tea.Cmd) {
m.restore.branch = msg.branch m.restore.branch = msg.branch
} }
m.restore.loss = msg.loss m.restore.loss = msg.loss
// a hard restore is asked for to drop staged pins: alone they are no if msg.loss.Empty() {
// reason to ask, but are listed if something else is lost
lost := msg.loss
if m.restore.to == submodule.RestoreHard {
lost.StagedPins, lost.Modules = 0, 0
}
if lost.Empty() {
return m.runRestore() return m.runRestore()
} }
m.restore.phase = restoreAsking m.restore.phase = restoreAsking
+12 -5
View File
@@ -189,7 +189,7 @@ func TestRestoreOfAStagedPinAsksWhichPin(t *testing.T) {
} }
} }
func TestHardRestoreOfAStagedPinDoesNotAsk(t *testing.T) { func TestHardRestoreAsksToDropStagedPins(t *testing.T) {
m, f := newRestoreModel(t, submodule.Loss{StagedPins: 2, Modules: 2}) m, f := newRestoreModel(t, submodule.Loss{StagedPins: 2, Modules: 2})
mm := m.(Model) mm := m.(Model)
var staged *submodule.Submodule var staged *submodule.Submodule
@@ -202,15 +202,22 @@ func TestHardRestoreOfAStagedPinDoesNotAsk(t *testing.T) {
mm.tree.Select(staged.Path) mm.tree.Select(staged.Path)
m = mm m = mm
// neither which pin, nor y/n for staged pins alone // not which pin, but y/n: staged pins alone are a reason to ask
m, cmd := press(m, "X") m, cmd := press(m, "X")
var next tea.Cmd
for _, msg := range run(cmd) { for _, msg := range run(cmd) {
if l, ok := msg.(lossMsg); ok { if l, ok := msg.(lossMsg); ok {
m, next = m.Update(l) m, _ = m.Update(l)
} }
} }
feed(m, next) r := m.(Model).restore
if r == nil || r.phase != restoreAsking {
t.Fatalf("want a question, got %+v", r)
}
if got := lossText(r.loss); got != "2 staged pins" {
t.Errorf("question lists %q, want the staged pins", got)
}
m, cmd = press(m, "y")
feed(m, cmd)
if len(f.to) != 1 || f.to[0] != submodule.RestoreHard || f.restored[0] != staged.Path { if len(f.to) != 1 || f.to[0] != submodule.RestoreHard || f.restored[0] != staged.Path {
t.Fatalf("restored %q to %v, want %s hard", f.restored, f.to, staged.Path) t.Fatalf("restored %q to %v, want %s hard", f.restored, f.to, staged.Path)
} }