fix(install): bottom-anchored progress bar + real byte tracking#77
Merged
Conversation
The previous top-reserved scroll region put the cask progress bar at the very top of the terminal viewport — far from where the user typed the command, and persisting as visual residue after the bar was released. Both behaviors were jarring for a one-shot CLI command. Mirror tmux's bottom status bar: reserve the LAST two rows so the bar appears near the user's eye-line, and clear those rows on reset so no residue carries into subsequent prompts. formatLines reorders to [divider, status] so the divider sits ABOVE the bar visually.
…ts help
The original PR called `brew info --json --cask <name>` which brew silently
treats as malformed: stdout gets the help text (exit 0), so json.Unmarshal
fails, FetchCaskSizes returns an empty map, and the cache tracker is never
spawned — leaving bytes/speed/ETA stuck on '—' through every cask install.
The correct invocation is `--json=v2`, which returns the documented
{"formulae": [...], "casks": [...]} envelope. Adjusted the unmarshal
target to match.
Why tests didn't catch it: both the unit fake (`withFakeBrew`) and the
integration shell fake matched on `$1 == 'info' && $2 == '--json'` and
returned canned JSON regardless of brew's actual CLI grammar. The fakes
documented our buggy expectations instead of brew's reality.
Count-based bar lies when casks vary in size by 10x. For a mix of rectangle (4MB) + iina (104MB), finishing rectangle pushes the bar to 50% — but real work remaining is 96%. Track aggregate byte progress across the cask phase: - phaseTotalBytes seeded once from FetchCaskSizes sum - phaseCompletedBytes accumulated as each cask finishes - pctForBar = (phaseCompletedBytes + currentBytes) / phaseTotalBytes - AddCompletedCaskBytes resets per-cask state but keeps speed EMA so the network estimate carries across casks - Fall back to count-based when phaseTotalBytes == 0 (all HEAD failures) - Formula phase stays count-based — formulae are similar size and the count signal is the right one there
The bar previously used count-based progress for formula and byte-based for cask, with a jump at the phase transition. Worse: count-based bar lies when packages vary in size — finishing 5 small formulae pushes the bar to 50% while most real work is still in the few large casks. Single bar across the whole install: - FetchFormulaSizes HEADs each formula's bottle URL via httputil.HeadWithBearer (GHCR anonymous token "QQ==") - FetchCaskSizes unchanged - InstallWithProgress pre-fetches both, sums them, calls SetTotalBytes once - Both runSerialInstallWithProgress and installCasksWithProgress receive the relevant sizes map and call AddCompletedBytes on each success - pctForBar uses installCompletedBytes + currentBytes / installTotalBytes unconditionally — phase is no longer gated - Phase still affects the HEAD display (formula shows count only, cask shows live bytes/speed/ETA) since formulae have no live tracker, only lump-sum size on completion Renames for clarity: - phaseTotalBytes → installTotalBytes - phaseCompletedBytes → installCompletedBytes - SetPhaseBytesTotal → SetTotalBytes - AddCompletedCaskBytes → AddCompletedBytes
…ot '—' Tracker first poll lands ~500ms after the cask starts. In that window the head used to show '—' for bytes because both currentBytes and totalBytes were 0. Seeding totalBytes from the known size right after SetCurrent makes the head jump to '0B/<size>' immediately, then the tracker takes over with real values.
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
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.
Summary
Follow-up to #76 fixing three real problems caught during manual smoke testing:
Scroll region was anchored to the top of the viewport, far from where the user typed the command. Mirrored tmux's bottom status bar: reserve the LAST two rows so the bar appears near the user's eye-line.
reset()now also clears the reserved rows so the bar doesn't leave residue in the scrollback.Cask bytes/speed/ETA never updated — stuck on
—for every cask.brew info --json --cask <name>prints help text (exit 0) instead of JSON; the correct invocation is--json=v2. The fake brew runner in tests matched our buggy expectation instead of brew's actual CLI grammar, hiding the bug. Switched to--json=v2and updated both fakes to mirror reality.Bar was count-based. For a mix of small formulae (5MB) + large casks (104MB), the bar would jump to "50% done" after the small ones finished while most real work remained. Replaced with byte-based progress across the WHOLE install (formula + cask, single continuous bar). Formula bottle sizes are HEAD'd via GHCR's anonymous Bearer token (
QQ==, the same trick brew uses internally) — a newhttputil.HeadWithBearerkeeps HTTP construction insideinternal/httputil/per archtest.Plus one small polish: at each cask entry, seed
totalBytesfrom the known size so the head shows0B/<size>immediately instead of—for the ~500ms before the cache tracker's first poll.Test plan
make test-unit) green across all packages-raceclean oninternal/brewandinternal/uibrew_install.goafter the new sizes pre-fetch block)HeadWithBearersends the Authorization header;FetchFormulaSizeshappy + missing-bottle paths;pctForBarbyte-based across phases + count-based fallback + Phase reset semantics[7/10] alt-tab≈ 10% (formulae + 2 small casks ≈ 21MB of 174MB)[8/10] keka 35M/35M≈ 38% (3 casks + formulae done, keka downloaded)Process note
Both problem 1 (scroll region placement) and problem 2 (
--jsonflag) slipped through #76's review because the test fakes documented our buggy expectations instead of brew's reality. The fakes only checkedargs[1] == "--json"— they didn't validate brew would accept that combination. Real brew rejects it. Lesson logged for future PRs that touch brew CLI grammar: at least one integration test must fork realbrew(or the contract test suite must cover the call shape).