feat: widen tables to 100 columns so the detail column is readable - #90
Merged
Conversation
render_table squeezes the single widest column to fit its budget, and that column is always the free-text one — so every column the budget spent elsewhere came straight out of the workout description, the part actually worth reading. Plan rows were wrapping detail at ~17 characters. Fixed rather than read from $COLUMNS on purpose: env-dependent output would make the e2e layout assertions unreproducible. The width expect now asserts against the constant instead of a literal 80, which is why it failed on a change that was purely the number moving.
There was a problem hiding this comment.
Pull request overview
This PR increases Render.render_table’s fixed maximum table width from 80 to 100 display columns so the widest free-text column (e.g., detail / workout names) is less aggressively squeezed and wraps less, improving readability while keeping output deterministic for e2e layout assertions.
Changes:
- Increase
Render.max_totalfrom 80 → 100 and expand the rationale in the module comment (including why it is intentionally not$COLUMNS-adaptive). - Update the table-width test to assert against
Render.max_totalinstead of hard-coding<= 80.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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.
Why
render_tablekeeps the whole table inside a budget by squeezing the single widest column. That column is always the free-text one —detailon the plan, the workout name elsewhere — so every column the budget spends elsewhere comes straight out of the text actually worth reading.At 80 the plan wrapped detail at roughly 17 characters:
At 100:
Fixed, not adaptive
Deliberately not read from
$COLUMNS. It would look nicer per-terminal and make output environment-dependent, which breaks the e2e assertions that pin exact layout — including the one catching a progress bar wrapped mid-bar.One test changed
The width expect hard-coded
<= 80, so it failed on a change that was purely the number moving. It now asserts<= Render.max_total, expressing the invariant rather than repeating the constant. Everything else passes untouched: 202 e2e checks green, including the bar-never-wraps and marker-placement assertions that are most sensitive to width.