Replace mpb progress bars with tview TUI#72
Open
RyanAMayers wants to merge 8 commits into
Open
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request replaces the mpb progress bar library with a custom TUI implemented using tview and tcell. The new interface provides a comprehensive view of the migration process, including a status header, table-level progress tracking with ETA and transfer rates, and a scrollable log pane. The PR also introduces a --version flag, fixes several typos in flag names and descriptions, and refines the formatShort utility. Review feedback highlights the need for robust UTF-8 handling in string truncation and centering logic, and recommends using os.CreateTemp for the log file to prevent security risks and file collisions.
RyanAMayers
force-pushed
the
feat/tview-tui
branch
from
ce22d28 to
583051a
Compare
Member
BrianLeishman
left a comment
BrianLeishman
left a comment
There was a problem hiding this comment.
missing an update to the readme video!
RyanAMayers
force-pushed
the
feat/tview-tui
branch
from
06d403a to
2beec7f
Compare
Drops the mpb/v8-based progress rendering in favor of a full-screen tview TUI. Per-table state flows through atomics on a shared struct; a 100ms render tick snapshots state and issues one QueueUpdateDraw, so worker goroutines never touch the UI directly. Fixes the bar duplication caused by slog warnings interleaving with mpb cursor-up redraws. Layout is a vertical Flex: - 3-line header (version, source/dest, rollup counters, elapsed) - Scrollable body TextView rendering one row per table - Fixed-height log pane (min(10, h*30%)) tailing a ring buffer Per-row format: <glyph> <name> <pct> [=====>-----] counts rate state with the bar absorbing any remaining terminal width. Name and state text share the bar's status color. Fallback paths keep the legacy slog-to-stderr behavior: - -no-progress (explicit opt-out) - -verbose (per-query spam would flood the log pane) - non-TTY stdout (piped output) slog output is tee'd to /tmp/swoof.log (truncated each run) whenever the TUI is active, so transient retry details remain available for post-mortem review without cluttering the screen. Also in this commit: - Add -version flag for quick version checks - Pre-TUI run header listing source / destination / tables - Setup status lines during pool connect / table resolution - formatShort rounds to integers for 100+ values (100K, not 100.0K) - Retry warnings log full error + chain to file, short cause in TUI Deps: add rivo/tview, gdamore/tcell/v2, golang.org/x/term; drop vbauerster/mpb/v8 and its sub-packages.
Add per-table lifecycle logs (starting/finished/recovered) and a "swoof run started" / "table imports complete" pair so the log pane shows continuous activity instead of just retry warnings. Log lines in the pane render with severity-colored levels and dimmed attribute keys. Move the `swoof vX.Y.Z` title and the update-available banner ahead of the "connecting to source" setup status lines so the user sees what's running before any work starts. Override tview's color theme to use ColorDefault everywhere so the TUI inherits the terminal's bg/fg (works with light themes, custom palettes, transparent terminals). Explicit color tags continue to override per-text. Bug fixes from the pre-merge audit: - Worker-driven u.Fatal no longer prints "swoof: interrupted" with exit 130; an errInterrupted sentinel distinguishes user Ctrl+C/q from a permanent worker failure (the latter exits 1 with the real error). - state.Begin resets StartedAt and FinishedAt every call so retry rate/ETA reflects the current attempt rather than time accumulated across earlier tries; total wall-clock duration is still tracked via a local in the goroutine. - Drop the dead seed loop in newUI (left over from when body was a Table primitive). - Drop the write-only stopTick field from the ui struct. - Consolidate Fatal and requestStop; Ctrl+C/q now calls Fatal(errInterrupted) directly. - Color-tag syntax in the header summary made consistent: every bold value uses `[white::b]VALUE[-:-:-]` then `[gray]` resumes for the next static run.
Use os.CreateTemp for the TUI log instead of a hardcoded path in /tmp to avoid symlink and multi-user collision risks, and switch the header centering plus truncateRight/truncateMiddle to rune-based counting and slicing so multi-byte names render correctly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a per-table statusFinalized state alongside statusDone so the post-workers swap pass shows live progress (blue "imported" → green "done"), and holds the TUI open with a centered "Press Q to exit" prompt and green pane borders so the user can scan the final state and scroll the log before dismissing. Required refactoring the finalization phase (delayed temp-table swap, funcs/views/procs/clipboard imports) to return errors instead of calling os.Exit — a hard exit while tview still owns the screen would leave the alt-screen on top with no visible error. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Tightens the post-tview-migration UX a number of small ways:
- Three-line title with copyright + license, kept in scrollback
- TUI starts before connection setup so connect/resolve logs land in
the log pane; setup statuses + table list + source/dest summary
replay into terminal scrollback after the user dismisses
- Per-table finalize state: blue "imported" while waiting on the
delayed temp-table swap, green "done" once swapped + triggers copied
- Body shows a centered "Resolving tables on X..." placeholder during
the resolve phase, with a 1-2-3-dot ellipsis animation
- Both body and log panes carry a one-column right-edge scrollbar
(custom Draw overlay since tview v0.42.0 has no built-in for
TextView). Tab / Shift-Tab cycle focus between them; ▸ marker on
the focused pane's title; log auto-scroll-to-end pauses when the
log is focused
- Yellow→green border blink on completion before settling on green
- "Press Q to exit" pinned bold-yellow at the bottom of the log pane
- Slog messages tied to per-row lifecycle ("starting table",
"finished table", "finalized table") tinted to match the body's
status palette
- Finalization phase refactored to return errors so failures surface
cleanly instead of os.Exit'ing through the live alt-screen
- Re-renders keep firing in the completed state so terminal resizes
reflow correctly; SetText preserves scroll position
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The retrying glyph was U+23F3 ⏳ (hourglass), which is emoji-class and renders double-width on macOS Terminal and iTerm2 — pushing the row layout one column off alignment whenever a table was retrying. Replaced with U+21BB ↻ (clockwise open-circle arrow), narrow on every terminal. To keep the running and retrying states visually distinct, also swapped the running glyph from U+27F3 ⟳ (also a clockwise circle arrow, easily confused with the new retrying glyph) to U+25B6 ▶ (right-pointing triangle, the universal "play" symbol). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a desktop notification on swoof completion via gen2brain/beeep (silently no-ops on hosts without a notification service, including SSH-to-headless and CI). Trims comments across both files: removes docstrings that just restate function names, drops layout walkthroughs that the code itself shows, and tightens the remaining ones to keep only the non-obvious WHY (race avoidance, hidden constraints, MySQL quirks, etc.). Also drops postCompletionRendered now that the render loop runs forever post-completion — TextView.SetText preserves scroll so the gate was unnecessary, and removing it lets terminal resizes reflow correctly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
RyanAMayers
force-pushed
the
feat/tview-tui
branch
from
2721bec to
c9be19e
Compare
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
2 participants
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.
Summary
QueueUpdateDraw, fixing bar duplication caused by slog warnings interleaving with mpb cursor-up redraws./tmp/swoof.logwhile the TUI is active (truncated per run) so retry details survive post-mortem; fall back to legacy slog-to-stderr under-no-progress,-verbose, or non-TTY stdout.ColorDefaultoverrides so the TUI inherits the terminal's theme.errInterrupted, exit 130, "swoof: interrupted") from workerFatal(exit 1, real error); resetstate.Beginon retry so rate/ETA reflect the current attempt.-versionflag, pre-TUI run header (source / destination / tables), and integer rounding informatShortfor 100+ values.Test plan
-no-progress,-verbose, and piped stdout; confirm fallback to slog-to-stderr in each/tmp/swoof.logcontains full error chain-versionstandalone🤖 Generated with Claude Code