fix(widths): guarantee minimum column width of 1 (empty-column alignment) - #5
Merged
Merged
Conversation
markdown-table-wrap-distribute-widths had two paths with different minimum-width contracts: the shrinking path floored every column at 1, but the fits path returned raw natural widths, which are 0 for an all-empty column. Returning 0 misaligns renderers: the separator draws a dash (via its own max-1 floor) where the row draws nothing, so every line after the separator drifts by one character per empty column. Apply max 1 in the fits path too, so both paths agree that column widths are always >= 1. The existing test markdown-table-wrap-test-widths-fit-naturally already asserted widths >= 1; this makes the fits path actually honor that contract for empty columns. Adds two regression tests: an all-empty column computes to width 1 (not 0), and a rendered table with an all-empty column has rows and separator of equal width (no drift).
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.
Summary
markdown-table-wrap-distribute-widthshad two code paths with different minimum-width contracts. The shrinking path (table wider than available) already floored every column at 1 via(max 1 (floor ...)), but the fits path (the common case) returned raw natural widths — which are 0 for a column whose header and every cell are empty. Returning 0 misaligns the rendered table: the separator line draws a dash (via its own defensivemax 1) where the row draws nothing, so every line after the separator drifts by one character per empty column.This applies
(max 1 ...)in the fits path too, so both paths agree that column widths are always ≥ 1.Repro
A table with an all-empty middle column, rendered with
markdown-table-wrapat a wide terminal:Before (rows are 1 char narrower than the separator per empty column):
After (all lines aligned):
Why this is the right layer
The inconsistency is internal to
distribute-widths: the function already believes in a min-1 contract (the shrinking path enforces it), it just didn't apply it in the fits path. Every renderer (markdown-table-wrapitself, and downstream consumers like pi-coding-agent's chat and thetable-prettyoverlay library) has to defensively(max 1 w)on the separator but not the row (or vice versa), and they drift. Fixing the allocation function once fixes all renderers, and the separator's existing(max 1 w)is no longer patching over a 0 that shouldn't have been returned.The existing test
markdown-table-wrap-test-widths-fit-naturallyalready asserted(should (>= (nth N widths) 1))— the suite already encoded the min-1 contract; this just makes the fits path actually honor it for empty columns.Scope
markdown-table-wrap-distribute-widths(fits path): wrap the returned natural widths in(mapcar (lambda (w) (max 1 w)) ...).compute-table-metrics(natural-widths remain a pure measurement of max visible width).border-overhead,total-natural) — those use raw natural widths for the fits/shrink threshold, which is unaffected; the min-1 floor only applies to the returned widths.Behavior risk
None. A 0-width column cannot render at 0 anyway (it needs at least
| |); bumping to 1 is strictly more correct. The shrinking path is untouched (it already capped at natural and floored at 1). The sqrt weights already used(max 1.0 ...)so empty columns already participated with weight 1.0.Tests
Two new ERT tests in
test/markdown-table-wrap-test.el:markdown-table-wrap-test-widths-empty-column-min-1— an all-empty column computes to width 1 (not 0).markdown-table-wrap-test-empty-column-row-separator-align— a rendered table with an all-empty column has rows and separator of equal width (no drift).Full suite: 156 tests pass (154 existing + 2 new), 0 regressions. Byte-compile clean.