feat(wsh): add 'tab list' and 'tab move' commands for scriptable tab management#3426
feat(wsh): add 'tab list' and 'tab move' commands for scriptable tab management#3426Jason-Shen2 wants to merge 3 commits into
Conversation
… stderr bug - Add 'wsh tab list' command to list tabs with ids and names in current order - Add 'wsh tab move <tabid> --index N' command to reorder tabs - Both commands default to WAVETERM_WORKSPACEID, with --workspace override - Include --json output mode for scripting - Add unit tests for tab reorder logic (8 test cases covering edge cases) - Fix bug in 'wsh workspace list' where workspaceId was printed to stderr instead of stdout (WriteStderr -> WriteStdout) Closes wavetermdev#3425
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cmd/wsh/cmd/wshcmd-tab.go (1)
56-57: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider using
MarkFlagRequiredfor--index.The manual
cmd.Flags().Changed("index")check works, buttabMoveCmd.MarkFlagRequired("index")ininit()would enforce this at the Cobra layer with a standard error message beforeRunEis called.Also applies to: 159-161
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/wsh/cmd/wshcmd-tab.go` around lines 56 - 57, Update the tabMoveCmd initialization to mark the "index" flag required via Cobra's MarkFlagRequired in init(), and remove the manual cmd.Flags().Changed("index") validation from the command handler so missing --index is rejected before RunE executes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/wsh/cmd/wshcmd-tab.go`:
- Line 111: Initialize entries in the tab-list construction flow as a non-nil
empty []tabListEntry slice so an empty workspace marshals as [] rather than
null. Preserve the existing append and JSON marshaling behavior for workspaces
containing tabs.
---
Nitpick comments:
In `@cmd/wsh/cmd/wshcmd-tab.go`:
- Around line 56-57: Update the tabMoveCmd initialization to mark the "index"
flag required via Cobra's MarkFlagRequired in init(), and remove the manual
cmd.Flags().Changed("index") validation from the command handler so missing
--index is rejected before RunE executes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2d929a98-9e05-4feb-ab8d-424f55a5060a
📒 Files selected for processing (3)
cmd/wsh/cmd/wshcmd-tab.gocmd/wsh/cmd/wshcmd-tab_test.gocmd/wsh/cmd/wshcmd-workspace.go
- Initialize entries slice as non-nil to marshal empty workspaces as [] instead of null - Use MarkFlagRequired for --index flag instead of manual Changed() check
|
Addressed both CodeRabbit review comments in commit b9e0e68:
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cmd/wsh/cmd/wshcmd-tab.go (1)
112-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFix the ST1023 lint finding.
The explicit type is redundant because
makealready determines it. Use inferred declaration syntax while preserving the non-nil empty slice behavior.Proposed fix
- var entries []tabListEntry = make([]tabListEntry, 0) + entries := make([]tabListEntry, 0)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/wsh/cmd/wshcmd-tab.go` at line 112, Update the entries declaration in the tab-list construction to use inferred declaration syntax while retaining make’s tabListEntry element type and non-nil empty slice behavior.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@cmd/wsh/cmd/wshcmd-tab.go`:
- Line 112: Update the entries declaration in the tab-list construction to use
inferred declaration syntax while retaining make’s tabListEntry element type and
non-nil empty slice behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 6efcdc12-e213-4864-b67b-82d6620c0811
📒 Files selected for processing (1)
cmd/wsh/cmd/wshcmd-tab.go
1 participant
Summary
This PR adds two new wsh subcommands under
wsh tabto make tabs scriptable from the command line, addressing #3425.New commands
wsh tab list(alias:ls) — lists tabs in the current workspace with their ids, names, and indices.WAVETERM_WORKSPACEID, overridable with--workspace <id>--jsonflag for machine-readable outputwsh tab move <tabid> --index <n>— moves a tab to the specified 0-based position within its workspace.Bug fix
wsh workspace listwhere theworkspaceIdfield was being printed to stderr instead of stdout due to aWriteStderr/WriteStdouttypo.Testing
reorderTabIds) covering 8 scenarios: first↔last, left/right moves, adjacent swaps, and two-element arrayscmd/wsh/cmd/continue to passgo build ./cmd/wsh/...andgo vet ./cmd/wsh/...cleangofmtverifiedCloses #3425