Count terminal columns instead of bytes when expanding tabs by feiiiiii5 · Pull Request #4018 · sharkdp/bat

feiiiiii5 · 2026-09-25T22:51:31Z

Description

expand_tabs advances its cursor with a byte offset and a byte length, but cursor is a terminal-column counter.

let mut text = &line[seq.index_of_start()..seq.index_past_end()];
while let Some(index) = text.find('\t') {
    // Add previous text.
    if index > 0 {
        *cursor += index;                      // byte offset
        buffer.push_str(&text[0..index]);
    }

    // Add tab.
    let spaces = width - (*cursor % width);    // column arithmetic
    *cursor += spaces;
    ...
}
*cursor += text.len();                         // byte length

str::find returns a byte offset and str::len is a byte count, so any multi-byte character before a tab inflates cursor by len_utf8 - width, and every later tab stop lands early.

cursor is cursor_total in printer.rs, and it decides how much padding each line gets:

let width = if cursor_total <= cursor_max {
    cursor_max - cursor_total + 1
} else { 0 };

so the emitted line is short by exactly the byte/column discrepancy and the --style=grid,numbers column shifts.

Reproduction

master at 4987f76709aae3a1c4db723c53874c9ddcb0c4fd, --tabs=4:

input	before	expected
`é\t`	`é` (1 space)	`é` (3 spaces)
`€\t`	`€`	`€`
`🦀\t`	`🦀` (no spaces at all)	`🦀`
`a\t🦀\t`	`a 🦀`	`a 🦀`

🦀 is four bytes and two columns, so on master the tab expands to nothing and the line ends up two columns short.

printer.rs already has the correct measurement for this quantity and explains why:

// Return the displayed width of a character.
fn char_width(c: char) -> usize {
    c.width().unwrap_or(if c.is_control() { 2 } else { 0 })
}

expand_tabs was not using it.

Fix

Measure with UnicodeWidthStr::width from the unicode-width crate bat already depends on, in the two places that advanced the cursor by a byte count.

Test plan

Three tests added to src/preprocessor.rs, next to the existing inline #[test] functions, using the file's existing style. They cover a multi-byte character with no tab, the tab stop after one and two columns of multi-byte text, and multi-byte text between two tabs, which exercises the text.len() line. Each pins an ASCII control case alongside it, so the tab-stop arithmetic itself cannot be broken while making the multi-byte cases pass.

$ cargo test --locked --lib preprocessor::

Before the fix, with only the tests added:

---- preprocessor::test_expand_tabs_counts_columns_not_bytes stdout ----
assertion `left == right` failed: "é" is 1 column(s)
  left: 2
 right: 1

test result: FAILED. 15 passed; 3 failed; 0 ignored; 0 measured; 132 filtered out

After:

$ cargo test --locked --lib
test result: ok. 150 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

$ cargo test --locked --test integration_tests
test result: ok. 261 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

$ cargo test --locked --test snapshot_tests
test result: ok. 27 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out

$ cargo fmt --check
$ cargo clippy --locked --lib -- -D warnings
    Finished `dev` profile [unoptimized + debuginfo] target(s)

The existing tab tests all read tests/examples/tabs.txt, which is pure ASCII, so they pass both before and after — they were never going to catch this.

Toolchain: 1.98.0, macOS.

feiiiiii5 · 2026-10-06T14:37:12Z

Hi sharkdp — #4018 has been open since 25 September with CI green (24 checks) and I have not had a reply yet, so this is the one nudge I planned.

The short version: expand_tabs in src/preprocessor.rs used str::len and the byte offset from str::find, but cursor counts terminal columns. Any multi-byte character before a tab inflates cursor by len_utf8 - width, so the tab stop lands early and the emitted line comes out short — which also shifts the --style=grid,numbers column. The fix measures with UnicodeWidthStr::width, and the CHANGELOG entry follows the see #4018 (@feiiiiii5) form the gate wants.

Reproduced on master at 4987f767 with --tabs=4: é<TAB> emits 3 columns instead of 4 and €<TAB> and 🦀<TAB> are off by one and two columns respectively. Two tests pin the single-character cases and the tab stop after multi-byte text.

Two things I could check from here if either is the blocker: whether you would rather this went through an issue first (CONTRIBUTING only asks for that on new features, so I read a tab-expansion fix as exempt), and whether unicode-width is acceptable as a dependency here — bat already uses it for the padding code in printer.rs, which is why I reached for it rather than counting grapheme clusters by hand.

Happy to close it if you would rather handle this one yourself.

MohammediYunus

Thanks for fixing this! I checked the CLI output at ea8c9ac against 4987f76. With tabs set to 4, the CJK and decomposed-accent cases below now reach the correct tab stop. Both were short on the base revision.

This small integration test complements the unit cases by exercising the printer path. It fails on the base and passes on this head, with no added dependency:

#[test]
fn tabs_4_unicode_columns() {
    bat()
        .arg("--tabs=4")
        .arg("--style=plain")
        .arg("--decorations=always")
        .arg("--color=never")
        .arg("--paging=never")
        .arg("--wrap=never")
        .write_stdin("界\tz\ne\u{301}\tz\n")
        .assert()
        .success()
        .stdout("界  z\ne\u{301}   z\n");
}

I also checked highlighted lines with TwoDark, a 20-column terminal width, and wrapping disabled. ASCII LF output stays at 20 columns; CRLF improves from 19 to 20. The tabs-disabled base/head comparisons in the same small input matrix were byte-identical.

Validation: macOS arm64, Rust 1.97.1, locked dependencies with minimal-application features. The added integration test was run with:

cargo test --locked --no-default-features --features minimal-application \
  --test integration_tests tabs_4_unicode_columns -- --exact

feiiiiii5 · 2026-10-08T15:41:36Z

Thanks for checking the CLI output and for writing that test — it goes through the printer end to end, which my unit tests in src/preprocessor.rs don't reach.

Added it unchanged in 8f8efd9, and confirmed both directions here: on this head cargo test --locked --no-default-features --features minimal-application --test integration_tests tabs_4_unicode_columns -- --exact passes, and on the base revision (4987f76) the same test fails with 界 z / é z — one and two columns short. The other 12 tabs integration tests and the 141 lib tests pass, and cargo fmt --check and cargo clippy --all-targets --all-features -- -D warnings are clean.

expand_tabs advances its cursor with the byte offset of a tab found by str::find and with str::len for the remainder, but the cursor is a column counter: it is compared against the terminal width to pad each line, and the stop itself is derived from it as width - cursor % width. Any multi-byte character before a tab therefore inflated the cursor by len_utf8 - width and every later tab stop landed early. "🦀\t" with --tabs=4 is four bytes and two columns, so the tab expanded to nothing and the line came out two columns short, which shifts the --style=grid column. printer.rs already measures characters with UnicodeWidthChar for exactly this reason; use the same crate here.

feiiiiii5 · 2026-10-09T12:48:56Z

Rebased onto master to clear the conflict. #4035 had taken the same slot at the top of the Bugfixes section, so the CHANGELOG.md conflict was resolved by keeping both entries; nothing else needed resolving.

Head is now e647283 and the diff is unchanged — CHANGELOG.md, src/preprocessor.rs, tests/integration_tests.rs, +62/−3.

Verified on the rebased head: cargo test --lib (150 passed), cargo test --test integration_tests (264 passed), cargo fmt --check and cargo clippy --all-targets clean.

feiiiiii5 added a commit to feiiiiii5/bat that referenced this pull request Sep 25, 2026

Add changelog entry for sharkdp#4018

ea8c9ac

MohammediYunus reviewed Oct 8, 2026

View reviewed changes

feiiiiii5 added 3 commits October 9, 2026 20:47

Add changelog entry for sharkdp#4018

a4a3f7e

Add an integration test for tab expansion after multi-byte text

e647283

feiiiiii5 force-pushed the fix/expand-tabs-columns branch from 8f8efd9 to e647283 Compare October 9, 2026 12:48

Merge branch 'master' into fix/expand-tabs-columns

ec3b8c4

Navigation Menu

Repository navigation

Count terminal columns instead of bytes when expanding tabs - #4018

Count terminal columns instead of bytes when expanding tabs#4018
feiiiiii5 wants to merge 4 commits into
sharkdp:masterfrom
feiiiiii5:fix/expand-tabs-columns

feiiiiii5 commented Sep 25, 2026

Uh oh!

feiiiiii5 commented Oct 6, 2026

Uh oh!

MohammediYunus left a comment

Uh oh!

feiiiiii5 commented Oct 8, 2026

Uh oh!

feiiiiii5 commented Oct 9, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

feiiiiii5 commented Sep 25, 2026

Description

Reproduction

Fix

Test plan

Uh oh!

feiiiiii5 commented Oct 6, 2026

Uh oh!

MohammediYunus left a comment

Choose a reason for hiding this comment

Uh oh!

feiiiiii5 commented Oct 8, 2026

Uh oh!

feiiiiii5 commented Oct 9, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants