Merged
Conversation
- Update dependencies to charm.land/bubbletea/v2 and lipgloss/v2 - Change View() signature from string to tea.View return type - Replace tea.WithAltScreen() option with view.AltScreen field - Remove tea.EnterAltScreen command from Init() method - Update tests to use new tea.KeyPressMsg interface - Add Windows build exclusions for riscv64 and arm architectures
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
WalkthroughUpdates TUI deps to charm.land v2, refactors the TUI to return and render tea.View (Init/AltScreen changes, nil program handling), updates tests for new key-press helpers and View content, switches CI to read Go version from go.mod, adds goreleaser ignore for windows/arm, and updates dependencies. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant RunTUI as "RunTUI (cmd)"
participant FS as "filesystem"
participant Model
participant Runner
participant Program as "Program (maybe nil)"
Note over RunTUI,Program: High-level TUI startup and run flow
Caller->>RunTUI: Call RunTUI(dir, config, log, filesystem, runner)
RunTUI->>FS: filesystem.ListBinaries(dir)
FS-->>RunTUI: []binaries
RunTUI->>Model: New model with log, filesystem, binaries
RunTUI->>Runner: runner.Run(model)
Runner-->>RunTUI: Program or nil
alt Program is nil
RunTUI-->>Caller: return (early, test/mock path)
else Program returned
RunTUI->>Program: Start program.Run()
Program-->>RunTUI: exit/status
RunTUI-->>Caller: return
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
- Configure setup-go action to read Go version from go.mod file - Remove hardcoded GO_VERSION environment variables - Remove Go version matrix from test workflow
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #365 +/- ##
==========================================
+ Coverage 69.68% 70.11% +0.42%
==========================================
Files 6 6
Lines 353 358 +5
==========================================
+ Hits 246 251 +5
Misses 97 97
Partials 10 10
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
.goreleaser.yaml (1)
19-23: Drop the unreachablewindows/riscv64ignore entry.
goarchdoes not currently includeriscv64, so this ignore rule never matches and can be misleading.✂️ Suggested cleanup
ignore: - - goos: windows - goarch: riscv64 - goos: windows goarch: arm🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.goreleaser.yaml around lines 19 - 23, Remove the unreachable ignore entry for goos: windows / goarch: riscv64 from the .goreleaser.yaml ignore list: the goarch value riscv64 is not recognized by Go toolchain so this rule never matches—delete that specific ignore mapping and keep only valid ignore entries (e.g., the existing windows/arm entry) so the ignore list reflects actual supported GOARCH values.internal/cli/tui.go (1)
304-393: SplitView()into smaller render helpers.This method now carries multiple responsibilities (empty-state handling, grid rendering, footer/layout composition, and view wrapping), which raises maintenance cost and test surface.
♻️ Refactor sketch
func (m *model) View() tea.View { - if len(m.choices) == 0 { - view := tea.NewView("No binaries found.\n") - view.AltScreen = true - return view - } - // styles + grid + layout + wrap - ... + if len(m.choices) == 0 { + return m.renderEmptyView() + } + return m.renderChoicesView() } + +func (m *model) renderEmptyView() tea.View { + view := tea.NewView("No binaries found.\n") + view.AltScreen = true + return view +} + +func (m *model) renderChoicesView() tea.View { + // style setup, grid construction, footer/layout composition + // then wrap with tea.NewView(...) and AltScreen=true +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cli/tui.go` around lines 304 - 393, The View() method on model is doing too many things; split it into smaller render helpers to improve readability and testability: extract empty-state handling into renderEmptyView(), grid construction into renderGrid() (use m.choices, m.rows, m.cols, m.cursorX/Y, colWidthPadding, visibleLenPrefix), status/footer composition into renderFooter() (use m.status, m.styles and footer text), and the final layout/boxing into renderContent(width,height) that applies lipgloss styles and padding; then have View() call these helpers (renderEmptyView()/renderGrid()/renderFooter()/renderContent()) and return the resulting tea.View so behavior remains unchanged while each helper can be unit-tested.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.goreleaser.yaml:
- Around line 19-23: Remove the unreachable ignore entry for goos: windows /
goarch: riscv64 from the .goreleaser.yaml ignore list: the goarch value riscv64
is not recognized by Go toolchain so this rule never matches—delete that
specific ignore mapping and keep only valid ignore entries (e.g., the existing
windows/arm entry) so the ignore list reflects actual supported GOARCH values.
In `@internal/cli/tui.go`:
- Around line 304-393: The View() method on model is doing too many things;
split it into smaller render helpers to improve readability and testability:
extract empty-state handling into renderEmptyView(), grid construction into
renderGrid() (use m.choices, m.rows, m.cols, m.cursorX/Y, colWidthPadding,
visibleLenPrefix), status/footer composition into renderFooter() (use m.status,
m.styles and footer text), and the final layout/boxing into
renderContent(width,height) that applies lipgloss styles and padding; then have
View() call these helpers
(renderEmptyView()/renderGrid()/renderFooter()/renderContent()) and return the
resulting tea.View so behavior remains unchanged while each helper can be
unit-tested.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (6)
.goreleaser.yamlgo.modinternal/cli/cli_test.gointernal/cli/mocks/ProgramRunner.gointernal/cli/tui.gointernal/cli/tui_test.go
- Simplify project description in README - Update mockery configuration to use latest version - Add ./bin directory to gitignore - Remove redundant windows/riscv64 ignore from goreleaser
- Add gocritic and revive configurations to .golangci.yaml - Rename parameters to avoid package name conflicts - Refactor test assertions to use literal path strings - Apply consistent formatting with blank lines and append patterns
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
internal/cli/tui.go (1)
309-399: RefactorView()into smaller helpers to reduce complexity.This function currently combines multiple responsibilities (state branches, grid rendering, layout assembly, and view wrapping). Splitting it will make future UI changes safer and easier to test.
♻️ Refactor sketch
func (m *model) View() tea.View { - if len(m.choices) == 0 { - view := tea.NewView("No binaries found.\n") - view.AltScreen = true - - return view - } - // styles + grid + layout + footer + wrapper... - view := tea.NewView(content) - view.AltScreen = true - - return view + if len(m.choices) == 0 { + return m.newAltScreenView("No binaries found.\n") + } + return m.newAltScreenView(m.renderMainContent()) } + +func (m *model) newAltScreenView(content string) tea.View { + view := tea.NewView(content) + view.AltScreen = true + return view +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cli/tui.go` around lines 309 - 399, The View method on model is doing too much—split it into smaller helpers to reduce complexity: extract the empty-case into renderEmptyView(), the grid building loop into buildGrid() or renderGrid(rows, cols, choices, cursorX, cursorY, colWidth, styles), the header/status/footer assembly into renderHeader(status, styles)/renderFooter(styles) and the final layout/padding into renderContent(width, height, leftPadding, totalHeightBase); update model.View to call these helpers (renderEmptyView, renderGrid/buildGrid, renderHeader, renderFooter, renderContent) so each helper has a single responsibility and the main View only composes their results.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.gitignore:
- Line 5: Replace the invalid ignore pattern "./bin" with a root-relative or
directory pattern such as "/bin/" (or "bin/") in the .gitignore so generated
binaries are reliably ignored; update the entry that currently reads "./bin" to
"/bin/".
In `@internal/fs/fs_test.go`:
- Around line 74-75: Tests in internal/fs/fs_test.go use hardcoded forward-slash
paths like "/go/bin" which break on Windows; replace those literal expectations
with platform-aware constructions (e.g., use filepath.Join("go", "bin") or
filepath.FromSlash("/go/bin")) wherever you see the hardcoded strings (instances
of "/go/bin" and other "/"-separated expectations around the spans mentioned) so
the tests assert OS-normalized paths; import "path/filepath" in the test file if
not already present.
---
Nitpick comments:
In `@internal/cli/tui.go`:
- Around line 309-399: The View method on model is doing too much—split it into
smaller helpers to reduce complexity: extract the empty-case into
renderEmptyView(), the grid building loop into buildGrid() or renderGrid(rows,
cols, choices, cursorX, cursorY, colWidth, styles), the header/status/footer
assembly into renderHeader(status, styles)/renderFooter(styles) and the final
layout/padding into renderContent(width, height, leftPadding, totalHeightBase);
update model.View to call these helpers (renderEmptyView, renderGrid/buildGrid,
renderHeader, renderFooter, renderContent) so each helper has a single
responsibility and the main View only composes their results.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.gitignore.golangci.yaml.goreleaser.yaml.mockery.yamlREADME.mdcmd/root.gointernal/cli/tui.gointernal/cli/tui_test.gointernal/fs/fs.gointernal/fs/fs_test.go
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/cli/tui_test.go
- Replace hardcoded Unix-style paths with filepath.Join() calls - Update test cases for DetermineBinDir and AdjustBinaryPath - Ensure Windows compatibility for path separators
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Nick Fedor <nick@nickfedor.com>
- Replace filepath.Join with filepath.FromSlash for OS-agnostic paths - Ensure correct path separators on Windows and Unix systems
…icholas-fedor/go-remove into fix/bubbletea-v2-api-migration
This was referenced Feb 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The following PR migrates the project to use the Bubble Tea v2 API.
Problem
Dependency updates introduced breaking API changes in Bubble Tea v2 that caused compilation failures throughout the codebase.
Solution
Updated all TUI-related code to conform to Bubble Tea v2's new API
specifications while maintaining existing functionality.
Changes
Summary by CodeRabbit
Chores
Refactor
Tests
Documentation