💔 Bye Bye DevMode TUI #344
Conversation
WalkthroughThis update removes the custom TUI-based UI and logger infrastructure from the development server workflow, simplifying logging and user interaction. All TUI abstractions are deleted, and the system now uses a global logger and standard output for logs and status messages. A new info box generator is introduced for displaying URLs. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant DevCmd
participant Server
participant Logger
User->>DevCmd: Run dev command
DevCmd->>Logger: Log startup info
DevCmd->>Server: Connect()
Server-->>DevCmd: Connection result
DevCmd->>Logger: Log build/watch/restart events
DevCmd->>Server: GenerateInfoBox()
DevCmd->>User: Print info box to stdout
DevCmd->>Logger: Log shutdown/teardown
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
internal/dev/server.go (1)
608-633: Make the info-box width adaptive to terminal size.Hard-coding
Width(100)risks ugly wrapping on narrow terminals and wasted space on wide ones.
Lipgloss can query the TTY width (lipgloss.Width/lipgloss.TerminalWidth()) allowing:w, _ := lipgloss.Size() if w == 0 || w > 100 { w = 100 } // fallback / cap devmodeBox := lipgloss.NewStyle().Width(w)...Consider this small tweak so the pretty box stays pretty across environments.
cmd/dev.go (1)
100-111: Spinner output mixes withos.Stdoutfrom the bundler.
bundler.Bundlestreams its log output toos.Stdout, whiletui.ShowSpinneris rendering the animated spinner on the same file descriptor. The two streams can interleave, causing scrambled output.If keeping the spinner, route the bundler output to an in-memory buffer (shown when done) or to
tui.SpinnerWriter()(if the lib provides one). Otherwise drop the spinner for the build step.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (6)
cmd/dev.go(1 hunks)go.mod(1 hunks)internal/dev/pending_logger.go(0 hunks)internal/dev/server.go(4 hunks)internal/dev/tui.go(0 hunks)internal/dev/tui_logger.go(0 hunks)
💤 Files with no reviewable changes (3)
- internal/dev/pending_logger.go
- internal/dev/tui_logger.go
- internal/dev/tui.go
🧰 Additional context used
🪛 golangci-lint (1.64.8)
internal/dev/server.go
600-600: var runningColor is unused
(unused)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build and Test (macos-latest)
- GitHub Check: Test CLI Upgrade Path (windows-latest)
🔇 Additional comments (2)
internal/dev/server.go (1)
572-579: Channel-closing hazard inConnect().
Connect()(caller) → reads froms.connectedand immediatelyclose(s.connected).
If, for any reason,Connect()were ever called a second time (e.g. future refactor, test helper) the second invocation would panic when it tries to close the already-closed channel. A tiny guard prevents a latent foot-gun:-err := <-s.connected -close(s.connected) +err := <-s.connected +select { +case <-s.connected: // drain if already closed +default: + close(s.connected) +}Or make
Connectidempotent by storing async.Oncearound the close.go.mod (1)
60-60: Confirm thex/termdependency can stay indirect.The module is now flagged as
// indirect. If nothing in your code importsgolang.org/x/term,go mod tidywill eventually drop it, otherwise Go will promote it back to direct. Rungo mod tidy && go test ./...to verify.
| defer watcher.Close(log) | ||
|
|
||
| tuiLogger.Trace("starting project server") | ||
| if err := projectServerCmd.Start(); err != nil { | ||
| errsystem.New(errsystem.ErrInvalidConfiguration, err, errsystem.WithContextMessage(fmt.Sprintf("Failed to start project: %s", err))).ShowErrorAndExit() | ||
| } | ||
| initRun := func() { |
There was a problem hiding this comment.
watcher.Close is invoked twice – second call may error.
You defer watcher.Close(log) right after the watcher is created and call the same method again inside teardown().
Unless (*Watcher).Close is explicitly idempotent this double-close can return an error or panic (e.g. “send on closed channel”). Remove one of the calls – the defer is usually sufficient.
- defer watcher.Close(log)
...
- teardown := func() {
- watcher.Close(log)
+ teardown := func() {
server.Close()
dev.KillProjectServer(log, projectServerCmd, pid)
}Also applies to: 230-233
🤖 Prompt for AI Agents
In cmd/dev.go around lines 172 to 174 and also lines 230 to 233, the
watcher.Close(log) method is called twice: once deferred immediately after
watcher creation and again inside the teardown() function. This double close can
cause errors or panics if Close is not idempotent. To fix this, remove the
explicit call to watcher.Close(log) inside teardown() and rely solely on the
deferred call to ensure the watcher is closed exactly once.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Ode to the Feature That Might Have Been
Oh darling thread of clever code,
A spark within the dev abode,
You promised grace, you whispered speed,
A perfect match for every need.
You shimmered bright in demos fair,
With animated flair to spare—
A marvel born of sleepless nights,
And countless merge conflicts and fights.
You danced through mock-ups, lived in dreams,
You dazzled all our beta teams.
But truth, that harsh and stubborn friend,
Declared your glory near its end.
The bugs crept in, the edge case grew,
Our unit tests all turned askew.
You crashed the build, you drained our will,
No patch, no hack could tame you still.
So now we part, you phantom gem,
Unshipped, unsung, like many them—
Yet in the changelogs you remain,
A ghost of joy, a hint of pain.
Farewell, sweet feature none shall see,
You almost were—but couldn't be.
Summary by CodeRabbit
Refactor
Chores