Skip to content

Add multi-tab support#1

Open
s4ros wants to merge 8 commits into
masterfrom
feature/tabs
Open

Add multi-tab support#1
s4ros wants to merge 8 commits into
masterfrom
feature/tabs

Conversation

@s4ros

@s4ros s4ros commented May 25, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adds multi-tab UI in k9s (ctrl-t open, ctrl-x close, ctrl-n/ctrl-b cycle, up to 9 tabs).
  • Each tab owns its view stack, command interpreter and navigation/filter histories; cluster connection and styles are shared.
  • Closes all other tabs on context switch to prevent outdated state from persisting.
  • Centralises namespace switching through App.switchNS, fanning out to every tab session.

Review-feedback fixes (commit Address tab review feedback)

  • Add multi-tab support #1 goroutine leak: activateSession now Stop()s the outgoing tab's top component before swapping pointers, so informers / port-forward tickers / watch loops don't accumulate per tab switch.
  • Refactor ScaleExtender + block scale-to-zero #2 panic in switchNS: nil-guard the GetTable() / GetModel() chain for non-*Browser viewers; mirrors the defensive pattern already used in Table.GetNamespace().
  • No output is displayed when failing derailed/k9s#3 mutex contradiction: sync.RWMutex removed. Every method is called from the tview main goroutine; the lock was providing false safety with TOCTOU windows. Contract now documented on the type and app.QueueUpdateDraw is the supported re-entry point.
  • Support /bin/bash as an SSH command derailed/k9s#4 orphan PageStack: newTab rolls back via sess.Content.Clear() when cmd.Init fails, so a failed session can't outlive the call as an attached primitive.
  • Is there a way to scroll when in log view?  derailed/k9s#5 newTabCmd empty-tab UX: gotoResource now returns the underlying error; newTabCmd unwinds the new tab via closeActive() when navigation fails so the user isn't left staring at a blank page after dismissing the error dialog.
  • Quick sorting/filtering derailed/k9s#6 defensive listener detach: CloseOtherTabs explicitly RemoveListeners Crumbs/Menu from each closing session before Clear(), protecting the "only active tab carries app-wide listeners" invariant against future regressions (the calls are no-ops today).

Note on scale_extender

The original branch bundled an unrelated ScaleExtender → dao.Scaler refactor. It has been extracted to a separate PR (see fix/scale-extender); the two revert commits at the tip of this branch neutralise it here so this PR contains tab work only.

Test plan

  • go build ./...
  • go vet ./internal/view/...
  • go test -race ./internal/view/... — full suite green (3 new tests in tab_manager_int_test.go cover Stop-on-switch, switchNS nil-guards, and post-mutex-removal state consistency)
  • Manual smoke in TUI: open ~5 tabs, round-trip switching, watch goroutine count via runtime.NumGoroutine()/pprof — stays flat
  • Trigger nav failure on ctrl-t (e.g., revoke RBAC for active resource) — tab unwinds, user lands back on previous tab
  • :ns switch with a YAML/detail view (non-Browser) open in any tab — no panic

🤖 Generated with Claude Code

s4ros and others added 8 commits April 29, 2026 19:10
K9s now supports multiple tabs to allow quick switching between
different views and resources. Users can open up to 9 tabs
simultaneously, with each tab maintaining its own isolated view stack,
command interpreter, and navigation/filter histories.

Key bindings have been added for tab management:
- `Ctrl-t`: Open a new tab pre-loaded with the current resource.
- `Ctrl-x`: Close the active tab (no-op if it's the last tab).
- `Ctrl-n`: Switch to the next tab, wrapping around.
- `Ctrl-b`: Switch to the previous tab, wrapping around.
When switching contexts, close all tabs except the currently active one
to
prevent outdated state from persisting.
This change centralizes the logic for switching namespaces within the
`App` struct. Previously, individual views directly called
`app.Config.SetActiveNamespace`, which has now been replaced by a call
to `app.switchNS`.

The `TabManager` now also has a `switchNS` method to update all relevant
sessions and their content when the namespace changes.
Introduces a unified `dao.Scaler` interface and implementation to handle
resource scaling logic, replacing direct type assertions. This
simplifies the code and makes it more robust when dealing with different
resource types.
Critical fixes:
- activateSession: stop the outgoing tab's top component before swapping
  to the incoming session, so informers, port-forward tickers and watch
  loops don't accumulate on every tab switch.
- switchNS: guard nil GetTable()/GetModel() chain so non-Browser viewers
  with uninitialised tables don't panic the TUI.
- Remove sync.RWMutex from TabManager: every method runs on the tview
  main goroutine, so the lock was providing false safety with TOCTOU
  windows between Lock/Unlock/act sequences.
- newTab: roll back the just-initialised PageStack when cmd.Init fails so
  the failed session doesn't outlive the call.

Minor fixes:
- newTabCmd: unwind the new tab via closeActive() when gotoResource fails,
  so the user isn't left on a blank tab after dismissing the error dialog.
  gotoResource now returns the underlying error.
- CloseOtherTabs: defensively detach Crumbs/Menu listeners from each
  closing session before Clear(), protecting against future regressions
  of the "only active tab has these listeners" invariant.

Regression tests in tab_manager_int_test.go cover the Stop-on-switch path,
the switchNS nil-table/nil-model paths, and post-mutex-removal state
consistency through close/close-other/round-trip sequences.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
When scaling a single resource, the dialog used to pre-fill the replicas
field with "0" whenever both the CRD scale subresource and the READY
column failed to yield a count. A user pressing OK without editing would
silently scale the workload to zero.

makeScaleForm now uses the empty string as an "unknown" sentinel (so a
legitimately scaled-to-zero workload still pre-fills 0) and returns an
error when neither read path succeeds, which the caller surfaces via the
flash bar. Bulk-scale keeps the documented "0 default, must type a
target" behaviour.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
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.

1 participant