diff --git a/modules/pipeline/logviewer.go b/modules/pipeline/logviewer.go index 20b158e..fe7da6c 100644 --- a/modules/pipeline/logviewer.go +++ b/modules/pipeline/logviewer.go @@ -408,6 +408,12 @@ func (m logViewModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { ss.cancel() } return m, tea.Quit + case "b": + for _, ss := range m.activeStreams { + ss.cancel() + } + m.ctx.UIWantBack = true + return m, tea.Quit case "s": if m.state == lvStateReady && m.selectedUUID != "" { node := m.selectedNode() @@ -875,9 +881,9 @@ func (m logViewModel) renderSplit(b *strings.Builder) { } // help line: left side is fixed, right side shows poll state / scroll % - helpLeft := " ↑/↓ select · l/d/i/o/tab tab · pgup/pgdn scroll · r refresh · q quit" + helpLeft := " ↑/↓ select · l/d/i/o/tab tab · pgup/pgdn scroll · r refresh · b back · q quit" if m.activeTab == tabLogs || m.activeTab == tabInputs || m.activeTab == tabOutputs { - helpLeft = " ↑/↓ select · l/d/i/o/tab tab · pgup/pgdn scroll · r refresh · s save · q quit" + helpLeft = " ↑/↓ select · l/d/i/o/tab tab · pgup/pgdn scroll · r refresh · s save · b back · q quit" } var helpRight string diff --git a/pkg/cmdctx/cmdctx.go b/pkg/cmdctx/cmdctx.go index 52ecdbb..90bc7af 100644 --- a/pkg/cmdctx/cmdctx.go +++ b/pkg/cmdctx/cmdctx.go @@ -237,9 +237,17 @@ type Ctx struct { // UIHistory is the --ui back-navigation stack: one UILink pushed per Hop // (link/up/view), popped by the "b" key. Session-lifetime only. UIHistory []UILink - // RestoreListPos is the cursor row (within the first-loaded page) to seed a - // freshly built table with when replaying a popped UILink's ListPos. - RestoreListPos int + // RestoreOffset is the absolute row index to restore when replaying a + // popped UILink's Offset. It is resolved against the current pageSize + // (page = RestoreOffset / pageSize, cursor = RestoreOffset % pageSize), + // since the terminal may have been resized since it was captured. + RestoreOffset int + // UIWantBack is read by finishUIExit right after a "view" ui_command's + // handler returns, to decide whether to pop UIHistory and redraw the caller + // (true) or quit outright (false, the default). A view handler that wants + // its screen's own "b" key to behave like the rest of --ui's back + // navigation must set this to true before returning — it is not automatic. + UIWantBack bool } // ScopedAuth returns Auth adjusted for Level: "org" clears ProjectID, "account" diff --git a/pkg/cmdctx/uilink.go b/pkg/cmdctx/uilink.go index fa52e6d..4d100dc 100644 --- a/pkg/cmdctx/uilink.go +++ b/pkg/cmdctx/uilink.go @@ -28,8 +28,11 @@ type UILink struct { Profile, Org, Project string FlagValues map[string]any - Screen UIScreenKind - ListPos int + Screen UIScreenKind + // Offset is the absolute row index (page*pageSize + cursor row) the user + // was on when this Link was captured — resize-safe, unlike a raw page + // number or in-page cursor alone. + Offset int } // PushUILink appends link to the back-navigation stack (LIFO — PopUILink pops diff --git a/pkg/registry/buildctx.go b/pkg/registry/buildctx.go index b2ca474..24e3176 100644 --- a/pkg/registry/buildctx.go +++ b/pkg/registry/buildctx.go @@ -380,7 +380,7 @@ func buildLinkCtx(ctx *cmdctx.Ctx, link *cmdctx.UILink, targetCs *spec.CommandSp } } else { newCtx.ParentId = link.Id - newCtx.RestoreListPos = link.ListPos + newCtx.RestoreOffset = link.Offset } return newCtx, nil } diff --git a/pkg/registry/uitableview.go b/pkg/registry/uitableview.go index 3bfc428..b9a977f 100644 --- a/pkg/registry/uitableview.go +++ b/pkg/registry/uitableview.go @@ -134,8 +134,8 @@ type uiTableModel struct { width int height int - // listpos restore — seeded from ctx.RestoreListPos, applied once on the - // first page load only (later page loads always GotoTop as before). + // cursor restore — derived from ctx.RestoreOffset (against pageSize), applied + // once on the first page load only (later page loads always GotoTop as before). restoreCursor int restoreApplied bool } @@ -156,6 +156,8 @@ func newUITableModel( getCs *spec.CommandSpec, ) uiTableModel { pageSize := tableHeight(termHeight) + page := ctx.RestoreOffset / pageSize + restoreCursor := ctx.RestoreOffset % pageSize colDefs := placeholderColumns(tspec, termWidth) t := tui.NewTable(colDefs, pageSize, termWidth) @@ -184,7 +186,8 @@ func newUITableModel( hasSearch: hasSearch, getCs: getCs, uiCommands: uiCommands, - restoreCursor: ctx.RestoreListPos, + page: page, + restoreCursor: restoreCursor, } } @@ -345,7 +348,7 @@ func (m uiTableModel) Init() tea.Cmd { if m.detailOnly { return m.fetchDetail(m.detail.id) } - return m.fetchPage(0) + return m.fetchPage(m.page) } func (m uiTableModel) fetchPage(page int) tea.Cmd { @@ -1279,10 +1282,10 @@ func currentScreenLink(ctx *cmdctx.Ctx, fm uiTableModel) cmdctx.UILink { Project: project, FlagValues: fv, Screen: screen, - // ListPos is always captured from the underlying table, even mid detail-flip: + // Offset is always captured from the underlying table, even mid detail-flip: // "b" always resumes the list, never the detail overlay, and detailOnly // screens (Case 4) never populate fm.t, so its Cursor() is a natural 0 there. - ListPos: fm.t.Cursor(), + Offset: fm.page*fm.pageSize + fm.t.Cursor(), } return link } @@ -1310,8 +1313,12 @@ func finishUIExit(ctx *cmdctx.Ctx, fm uiTableModel) error { if err := ctx.Resolver.RunUIHandler(ctx, fm.launchUIHandlerFn); err != nil { return err } - if link, ok := ctx.PopUILink(); ok { - return dispatchLink(ctx, &link) + // UIWantBack defaults to false: unless the handler explicitly asked to go + // back (e.g. its own "b" key), quitting its screen quits outright. + if ctx.UIWantBack { + if link, ok := ctx.PopUILink(); ok { + return dispatchLink(ctx, &link) + } } return nil } diff --git a/pkg/registry/uitableview_test.go b/pkg/registry/uitableview_test.go index 817e40a..55eb921 100644 --- a/pkg/registry/uitableview_test.go +++ b/pkg/registry/uitableview_test.go @@ -121,7 +121,7 @@ func TestFinishUIExit_PushesLinkOnViewHop(t *testing.T) { } } -func TestCurrentScreenLink_CapturesListPosOnTable(t *testing.T) { +func TestCurrentScreenLink_CapturesOffsetOnTable(t *testing.T) { ctx := &cmdctx.Ctx{Verb: VerbList, Noun: "thing", ParentId: "parent-1"} table := tui.NewTable(nil, 5, 40) rows := make([]tui.Row, 10) @@ -133,12 +133,12 @@ func TestCurrentScreenLink_CapturesListPosOnTable(t *testing.T) { fm := uiTableModel{t: table} link := currentScreenLink(ctx, fm) - if link.ListPos != 4 { - t.Fatalf("ListPos = %d, want 4", link.ListPos) + if link.Offset != 4 { + t.Fatalf("Offset = %d, want 4", link.Offset) } } -func TestCurrentScreenLink_CapturesListPosEvenMidDetailFlip(t *testing.T) { +func TestCurrentScreenLink_CapturesOffsetEvenMidDetailFlip(t *testing.T) { // "b" always resumes the underlying list, never the detail overlay, so an // in-place detail flip (detailMode true, detailOnly false) over a table must // still capture that table's live cursor. @@ -153,19 +153,36 @@ func TestCurrentScreenLink_CapturesListPosEvenMidDetailFlip(t *testing.T) { fm := uiTableModel{t: table, detailMode: true, detailOnly: false} link := currentScreenLink(ctx, fm) - if link.ListPos != 4 { - t.Fatalf("ListPos = %d, want 4 (mid-flip should still capture the list cursor)", link.ListPos) + if link.Offset != 4 { + t.Fatalf("Offset = %d, want 4 (mid-flip should still capture the list cursor)", link.Offset) } } -func TestCurrentScreenLink_DetailOnlyScreenHasZeroListPos(t *testing.T) { +func TestCurrentScreenLink_DetailOnlyScreenHasZeroOffset(t *testing.T) { // Case 4 detail-only Hops never populate fm.t, so its Cursor() is naturally 0. ctx := &cmdctx.Ctx{Verb: VerbGet, Noun: "thing", Id: "child-1"} fm := uiTableModel{detailMode: true, detailOnly: true} link := currentScreenLink(ctx, fm) - if link.ListPos != 0 { - t.Fatalf("ListPos = %d, want 0 (detail-only screens have no underlying table)", link.ListPos) + if link.Offset != 0 { + t.Fatalf("Offset = %d, want 0 (detail-only screens have no underlying table)", link.Offset) + } +} + +func TestCurrentScreenLink_CapturesOffsetAcrossPages(t *testing.T) { + ctx := &cmdctx.Ctx{Verb: VerbList, Noun: "thing", ParentId: "parent-1"} + table := tui.NewTable(nil, 5, 40) + rows := make([]tui.Row, 10) + for i := range rows { + rows[i] = tui.Row{"x"} + } + table.SetRows(rows) + table.SetCursor(3) + fm := uiTableModel{t: table, page: 2, pageSize: 20} + + link := currentScreenLink(ctx, fm) + if link.Offset != 43 { + t.Fatalf("Offset = %d, want 43 (page 2 * pageSize 20 + cursor 3)", link.Offset) } } @@ -235,10 +252,12 @@ func TestFinishUIExit_ViewHop_ResumesLeftScreen(t *testing.T) { launchUIId: "child-1", launchUIHandlerFn: "noop_handler", } - // The handler pushes the screen it's leaving, runs "noop_handler" (returns nil), then - // pops that same entry back off to resume it via dispatchLink — which doesn't resolve - // against an empty Registry; the resulting error is expected and irrelevant here. Only - // the net stack effect (resume, not leak or exit) is under test. + // The handler pushes the screen it's leaving, runs "noop_handler" (returns nil), then sets + // ctx.UIWantBack (as a view handler's own "b" key would) so finishUIExit pops that same + // entry back off to resume it via dispatchLink — which doesn't resolve against an empty + // Registry; the resulting error is expected and irrelevant here. Only the net stack effect + // (resume, not leak or exit) is under test. + ctx.UIWantBack = true _ = finishUIExit(ctx, fm) if len(ctx.UIHistory) != 1 || ctx.UIHistory[0].Id != "prev-id" { @@ -246,6 +265,33 @@ func TestFinishUIExit_ViewHop_ResumesLeftScreen(t *testing.T) { } } +func TestFinishUIExit_ViewHop_QuitOnlyByDefault(t *testing.T) { + r := New() + r.RegisterWorkflow("noop_handler", func(*cmdctx.Ctx) error { return nil }) + ctx := &cmdctx.Ctx{ + Verb: VerbGet, + Noun: "thing", + Id: "child-1", + Resolver: r, + UIHistory: []cmdctx.UILink{{Verb: VerbGet, Noun: "thing", Id: "prev-id"}}, + } + fm := uiTableModel{ + detailOnly: true, + launchUIId: "child-1", + launchUIHandlerFn: "noop_handler", + } + // The handler returns nil without setting ctx.UIWantBack (the default), so the view + // hop's own push is left in place and there is no pop/resume — quitting the handler's + // screen quits outright rather than resuming the caller. + if err := finishUIExit(ctx, fm); err != nil { + t.Fatalf("finishUIExit: %v", err) + } + + if len(ctx.UIHistory) != 2 { + t.Fatalf("UIHistory len = %d, want 2 (view hop pushed, no pop without UIWantBack)", len(ctx.UIHistory)) + } +} + func TestFinishUIExit_NoHopNoPush(t *testing.T) { ctx := &cmdctx.Ctx{Verb: VerbList, Noun: "thing", Resolver: New()} fm := uiTableModel{} @@ -257,34 +303,66 @@ func TestFinishUIExit_NoHopNoPush(t *testing.T) { } } -func TestBuildLinkCtx_TableScreen_CarriesListPosToRestoreListPos(t *testing.T) { +func TestBuildLinkCtx_TableScreen_CarriesOffsetToRestoreOffset(t *testing.T) { ctx := &cmdctx.Ctx{Context: context.Background(), Resolver: New()} - link := &cmdctx.UILink{Verb: VerbList, Noun: "thing", Id: "parent-1", Screen: cmdctx.ScreenTable, ListPos: 4} + link := &cmdctx.UILink{Verb: VerbList, Noun: "thing", Id: "parent-1", Screen: cmdctx.ScreenTable, Offset: 44} targetCs := &spec.CommandSpec{Verb: VerbList, Noun: "thing", NoAuth: true} newCtx, err := buildLinkCtx(ctx, link, targetCs) if err != nil { t.Fatalf("buildLinkCtx: %v", err) } - if newCtx.RestoreListPos != 4 { - t.Fatalf("RestoreListPos = %d, want 4", newCtx.RestoreListPos) + if newCtx.RestoreOffset != 44 { + t.Fatalf("RestoreOffset = %d, want 44", newCtx.RestoreOffset) } if newCtx.ParentId != "parent-1" { t.Fatalf("ParentId = %q, want parent-1", newCtx.ParentId) } } -func TestBuildLinkCtx_DetailScreen_DoesNotSetRestoreListPos(t *testing.T) { +func TestBuildLinkCtx_DetailScreen_DoesNotSetRestoreOffset(t *testing.T) { ctx := &cmdctx.Ctx{Context: context.Background(), Resolver: New()} - link := &cmdctx.UILink{Verb: VerbGet, Noun: "thing", Id: "child-1", Screen: cmdctx.ScreenDetailForGet, ListPos: 4} + link := &cmdctx.UILink{Verb: VerbGet, Noun: "thing", Id: "child-1", Screen: cmdctx.ScreenDetailForGet, Offset: 4} targetCs := &spec.CommandSpec{Verb: VerbGet, Noun: "thing", NoAuth: true} newCtx, err := buildLinkCtx(ctx, link, targetCs) if err != nil { t.Fatalf("buildLinkCtx: %v", err) } - if newCtx.RestoreListPos != 0 { - t.Fatalf("RestoreListPos = %d, want 0 (detail screens have no list cursor to restore)", newCtx.RestoreListPos) + if newCtx.RestoreOffset != 0 { + t.Fatalf("RestoreOffset = %d, want 0 (detail screens have no list cursor to restore)", newCtx.RestoreOffset) + } +} + +func TestNewUITableModel_SeedsPageAndCursorFromCtxRestoreOffset(t *testing.T) { + // termHeight 24 -> pageSize = tableHeight(24) = 24-uiOverheadLines-1. Derive + // the expected pageSize the same way the model does, so this test doesn't + // hardcode uiOverheadLines. + pageSize := tableHeight(24) + offset := 2*pageSize + 4 + ctx := &cmdctx.Ctx{RestoreOffset: offset} + m := newUITableModel(ctx, nil, nil, nil, nil, "title", 80, 24, nil) + if m.page != 2 { + t.Fatalf("page = %d, want 2 (derived from ctx.RestoreOffset / pageSize)", m.page) + } + if m.restoreCursor != 4 { + t.Fatalf("restoreCursor = %d, want 4 (derived from ctx.RestoreOffset %% pageSize)", m.restoreCursor) + } +} + +func TestNewUITableModel_RestoreOffsetSurvivesPageSizeChange(t *testing.T) { + // Capture at one pageSize, restore at a different (e.g. post-resize) + // pageSize, and confirm the absolute row is still targeted correctly. + capturedPageSize := 10 + capturedPage, capturedCursor := 2, 3 + offset := capturedPage*capturedPageSize + capturedCursor // absolute row 23 + + ctx := &cmdctx.Ctx{RestoreOffset: offset} + m := newUITableModel(ctx, nil, nil, nil, nil, "title", 80, 24, nil) + + restoredPageSize := m.pageSize + if got := m.page*restoredPageSize + m.restoreCursor; got != offset { + t.Fatalf("restored absolute row = %d, want %d (offset must survive pageSize change)", got, offset) } }