refactor(output): measure table cells once and fit columns in one flat pass - #316
Merged
Merged
Conversation
…t pass A table row is now a slice of cells, each a struct of its text, tail and compact form, measured once as the row goes in. add takes them whole; the tail and alt setters that patched the row just added, and the two maps keyed by row and column behind them, are gone. printTasks builds its tag, title and date cells and adds the row in one call. fit works on one state struct per column instead of three parallel bool slices and a mutated widths slice, and reads as the order it applies: drop the dropFirst columns, shrink to the soft floor, compact, drop, restore, shrink to the hard floor. Each step is a small primitive run by untilFits. - drop no longer calls the compact tier with a one-column list. - expand and regrow become restore, which always runs: the guard on whether anything was compacted or dropped never changed the result. - the soft and hard passes share shrinkTo and differ only in the floor. - measure sizes the column state to cover every column the lists name, so fit needs no bounds checks. Output is unchanged: the golden file is untouched, and a temporary differential test renders random tables, under the printers' own configurations and random ones, through both this and the previous implementation (kept as legacyTable in a test file) and compares every line at every width up to 130.
…helpers From review: cut takes the form lines already built, lines reads each row directly and pads only up to its last filled cell, softFloor drops a term its guard already covers, and form's flag no longer shadows the compact step.
ryanlewis
added a commit
that referenced
this pull request
Oct 1, 2026
…rom headers Print, PrintTaskList and PrintTaskWithChecklist read the layout once, a width and whether stdout is a terminal, and hand it to the printers. The two width policies stay as they were but are now visible where they are used: a task listing fits lay.width, which piped is termWidth's 120 fallback, and projects and the detail block fit lay.fitWidth(), which piped is 0. FitWidth reads the same value for the hint. printTasks is split into taskCells, the cells of one row, and groupHeaders, the blank line and group title above each row, so it only configures the table and interleaves the two. The header indent is a named constant. The legacy table and its differential test go: they proved the table rewrite in #316 and have no further use. TestTableFitNeverOverruns already pins the invariant worth keeping. Output is unchanged: the golden file is untouched.
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
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.
This is step two of the internal/output consolidation, following #314. It changes how
tableis put together and keeps every byte of output the same.What
fit()does now: it measures every column once, then runs one step after another, each stopping as soon as the row fits:dropFirstcolumns.dropOrdercolumns to their compact forms.dropOrdercolumns.restore).Before, the same order was spread across
compactTier,drop,expand,regrowand twoshrinkTocalls. Those passed three parallel[]boolslices and a widths slice that each step changed, and neededfreedflags, a guard on running expand and regrow, and bounds checks in four loops.Changes:
cell{text, tail, alt}, measured once when the row is added. Thetail()/alt()setters, and the[2]int-keyed maps behind them, are gone.printTasksbuilds its tag, title and date cells and adds the row in one call.col, in place of the parallel slices.dropandcompactare two small steps thatuntilFitsruns in turn, which leavesfit()at 13 lines (it was 26).expandandregroware merged intorestore, which always runs. The old guard on whether anything had been compacted or dropped never changed the result.shrinkToand differ only in their floor function.measurecreates state for every column the drop and shrink lists name, sofitneeds no bounds checks. A negative index is a bug and panics; before, it was ignored, and no printer uses one.padCol,joinWithGapanddropEmptySpansare unchanged.table.go goes from 429 to 388 lines, its non-comment code from 293 to 256, and its functions from 22 to 20.
How I checked the output is identical:
render_golden.txtis unchanged. Since test(output): pin piped, wide-rune, dim-cut and tab/newline title rendering #314 it covers piped output too.legacyTableintable_legacy_test.go.TestTableMatchesLegacy(seeded) andFuzzTableMatchesLegacyrender random tables through both and compare each line byte for byte at every width up to 130. The tables use the printers' own column setups and random ones. Cells cover dim text styled one character at a time, wide characters, tabs, newlines, tails, compact forms and short rows. I fuzzed for 10 minutes (272k runs), then another 3 minutes after the review changes, with no difference. Reversing the orderrestoreregrows columns in makes the test fail at once. The next PR removes both files.thingsfrom main and from this branch and ran both against a Things backup database, withHOMEpointed at a temp directory. The commands were today, inbox, upcoming, anytime, someday, repeating, logbook, trash, deadlines, projects (open and completed), areas, tags, two searches and twoshows. Each ran piped and under ascriptpseudo-terminal at 40, 62, 80, 100, 120 and 200 columns, with--color neverand--color always. All 238 pairs were byte-identical.scriptsometimes drops a\r, which happens even comparing main against itself, so pseudo-terminal output is compared with\rremoved; piped output is compared raw.make testandmake lintpass. While the differential test is in place, the output package's tests take about 19s under-race.