Skip to content

Fix table column alignment for ratio values#497

Merged
k0kubun merged 1 commit intoruby:mainfrom
k0kubun:fix-alignment
Mar 25, 2026
Merged

Fix table column alignment for ratio values#497
k0kubun merged 1 commit intoruby:mainfrom
k0kubun:fix-alignment

Conversation

@k0kubun
Copy link
Member

@k0kubun k0kubun commented Mar 25, 2026

Summary

  • Fix misaligned last column (before/after) in benchmark result tables
  • The ljust padding in format_ratio added trailing spaces to reserve room for significance symbols, but these were stripped by rstrip in the table formatter, breaking right-alignment of the last column
  • This was exposed by Fix pvalue display when --pvalue is not being passed. #492 which made pval nil when --pvalue is not passed, meaning the padding was always applied and then stripped in the common case

Before:

bench   before (ms)    after (ms)  after 1st itr  before/after
fib    160.3 ± 0.8%  160.5 ± 0.7%          0.924   0.998

After:

bench   before (ms)    after (ms)  after 1st itr  before/after
fib    160.3 ± 0.8%  160.5 ± 0.7%          0.924         0.998

The ljust padding in format_ratio added trailing spaces to reserve room
for significance symbols, but these spaces were then stripped by rstrip
in the table formatter's format_row, causing the last column to lose its
right-alignment.

This was exposed by ruby#492 which made pval nil when --pvalue is not passed,
meaning the significance symbol is always empty in the common case, yet
the ljust padding was still applied and then stripped.

Remove the ljust padding entirely and let the table formatter handle
column alignment, restoring the original format_ratio logic.
@k0kubun k0kubun marked this pull request as ready for review March 25, 2026 21:03
@k0kubun k0kubun requested review from a team and eightbitraptor March 25, 2026 21:03
@k0kubun k0kubun merged commit b33166a into ruby:main Mar 25, 2026
11 checks passed
@k0kubun k0kubun deleted the fix-alignment branch March 25, 2026 22:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants