Skip to content

Commit 88c9e49

Browse files
author
gluck
committed
cli: move before you kill, and the question leaves the list standing
Killing the aria on screen closed the socket the pager was watching, which ended the session whatever successor had been chosen. The successor is now attended FIRST and the old aria dies after, so the pager never holds a dead connection. The question also set the pit aside rather than closing it: answering no puts the listing back with its cursor where it stood.
1 parent 5f93955 commit 88c9e49

3 files changed

Lines changed: 61 additions & 13 deletions

File tree

‎internal/cli/command.go‎

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -387,31 +387,45 @@ func (in *interactiveInput) killAria(ctx context.Context, id string, recursive b
387387
if err != nil {
388388
return "", err
389389
}
390+
// MOVE BEFORE YOU KILL. The pager holds the subject's socket; killing the
391+
// aria it is watching closes that connection under the session, which
392+
// ends it whatever the successor was going to be. So the successor is
393+
// decided, attended, and only then does the old aria die.
390394
onScreen := id == in.currentID()
391-
successor := ""
392-
if onScreen {
393-
successor = in.successorFor(ctx, acli, id, prefer)
394-
}
395-
if err := acli.Kill(ctx, id, recursive); err != nil {
396-
return "", fmt.Errorf("kill %s: %w", id, err)
397-
}
398-
in.mu.Lock()
399-
in.jumps.forget(id)
400-
in.mu.Unlock()
401395
if !onScreen {
396+
if err := acli.Kill(ctx, id, recursive); err != nil {
397+
return "", fmt.Errorf("kill %s: %w", id, err)
398+
}
399+
in.forgetAria(id)
402400
return "killed " + id, nil
403401
}
402+
successor := in.successorFor(ctx, acli, id, prefer)
404403
if successor == "" {
404+
if err := acli.Kill(ctx, id, recursive); err != nil {
405+
return "", fmt.Errorf("kill %s: %w", id, err)
406+
}
407+
in.forgetAria(id)
405408
in.leaveSession()
406409
return "killed " + id + ": nothing left to attend", nil
407410
}
408411
note, aerr := in.attendNote(ctx, successor)
409412
if aerr != nil {
410413
return "", aerr
411414
}
415+
if err := acli.Kill(ctx, id, recursive); err != nil {
416+
return "", fmt.Errorf("kill %s: %w", id, err)
417+
}
418+
in.forgetAria(id)
412419
return "killed " + id + "; " + note, nil
413420
}
414421

422+
// forgetAria drops a dead aria from the path, so no hop can land on it.
423+
func (in *interactiveInput) forgetAria(id string) {
424+
in.mu.Lock()
425+
in.jumps.forget(id)
426+
in.mu.Unlock()
427+
}
428+
415429
// askKill puts the question up and runs the kill only on a yes. The question
416430
// is drawn by the pager, so this hops onto the render lock the way every
417431
// other pager mutation does.

‎internal/cli/confirm.go‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,25 @@ import "strings"
1010
// answer is swallowed) plus a pinned notice, which is the one alert that does
1111
// not retire on its own: it stands for exactly as long as the question does.
1212

13-
// confirmAsk is the pending question: the sentence, and what a yes runs.
13+
// confirmAsk is the pending question: the sentence, what a yes runs, and the
14+
// pit the question was asked from, which comes back either way. A reader who
15+
// says no must find their list exactly where they left it.
1416
type confirmAsk struct {
1517
prompt string
1618
yes func()
19+
behind pit
1720
}
1821

1922
// askConfirm puts the question up. The caller's action runs on the input
2023
// goroutine, so it must hand off anything that dials, exactly as every other
2124
// hook here does.
2225
func (t *transcript) askConfirm(prompt string, yes func()) {
23-
t.confirm = &confirmAsk{prompt: prompt, yes: yes}
26+
// THE PIT UNDERNEATH IS NOT CLOSED, it is set aside: showList would close
27+
// a hosted view and drop a list's cursor, and the question is a moment,
28+
// not a new place to be.
29+
behind := t.pit
30+
t.confirm = &confirmAsk{prompt: prompt, yes: yes, behind: behind}
31+
t.pit = pit{}
2432
t.pit.showList(pitConfirm, "", []pitRow{staticRow(" " + prompt + " [y/N]")})
2533
t.status.pinNotice(prompt)
2634
t.focused = focusPit
@@ -30,9 +38,13 @@ func (t *transcript) askConfirm(prompt string, yes func()) {
3038
// clearConfirm takes the question down and the notice with it, in that order:
3139
// the bar must not be left holding a sentence about a question nobody can see.
3240
func (t *transcript) clearConfirm() {
41+
ask := t.confirm
3342
t.confirm = nil
3443
if t.showing(pitConfirm) {
35-
t.pit.close()
44+
t.pit = pit{}
45+
if ask != nil {
46+
t.pit = ask.behind
47+
}
3648
}
3749
t.status.setNotice("")
3850
}

‎internal/cli/confirm_test.go‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,3 +69,25 @@ func TestConfirm_TheNoticeStandsWhileTheQuestionDoes(t *testing.T) {
6969
t.Fatalf("the notice must clear with the question: %q", note)
7070
}
7171
}
72+
73+
// The question is a moment, not a place: the list it was asked from is still
74+
// there afterwards, with its cursor where the reader left it.
75+
func TestConfirm_TheListSurvivesTheQuestion(t *testing.T) {
76+
tr, _ := visualFixture(t)
77+
tr.dropRow = func(pit, id, next string) {}
78+
tr.pit.showList(pitOutput, ":ls", []pitRow{
79+
{text: " aria1234", id: "aria1234", yank: "aria1234"},
80+
{text: " aria5678", id: "aria5678", yank: "aria5678"},
81+
})
82+
tr.focused = focusPit
83+
tr.pit.moveSelection(1) // stand on the second row
84+
tr.key('x')
85+
tr.key('n')
86+
if tr.pit.id != pitOutput {
87+
t.Fatalf("the listing did not come back: pit = %q", tr.pit.id)
88+
}
89+
row, ok := tr.pit.selected()
90+
if !ok || row.id != "aria5678" {
91+
t.Fatalf("the cursor moved across the question: %+v", row)
92+
}
93+
}

0 commit comments

Comments
 (0)