Skip to content

fix(ui): truncate long file paths so +/- stats stay visible - #4

Merged
bahdotsh merged 4 commits into
mainfrom
fix/truncate-long-file-paths
Mar 20, 2026
Merged

bahdotsh merged 4 commits into
mainfrom
fix/truncate-long-file-paths

Conversation

@bahdotsh

@bahdotsh bahdotsh commented Mar 20, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • Long file paths in the file list panel were pushing +N -N change stats off-screen, making them invisible
  • Truncate paths with an ellipsis (…) to guarantee the stats suffix always fits within the panel width
  • Uses char-boundary-safe slicing to avoid panics on non-ASCII paths, and a digit_count helper to measure stat widths without per-frame allocations

Test plan

  • cargo build passes
  • cargo test passes (71 tests)
  • Manually verify with a repo containing long file paths — stats should remain visible
  • Verify with a narrow terminal window — graceful degradation

The file list panel is 20% of terminal width. File paths were
rendered at full length with the +N -N change stats appended
after. Long paths simply shoved the stats off the right edge
into the void, making them invisible.

It turns out that rendering a path like
"src/some/deeply/nested/module/thing.rs +5 -2" into a 25-column
panel without any truncation is... optimistic.

Truncate paths with an ellipsis to guarantee the stats suffix
fits. Uses char-boundary-safe slicing (not byte indexing, because
panicking on non-ASCII paths would be embarrassing), and a
digit_count helper to measure stat widths without allocating
throwaway format strings every frame.
It turns out the original truncation was keeping the directory prefix
and chopping the filename — which is exactly backwards. Nobody cares
that a path starts with "src/components/"; they care that it ends
with "Button.tsx". Showing "src/components/d…" when you could show
"…/deep/Button.tsx" is just not helpful.

While at it, fix the edge case where max_name_width saturates to 0
or 1 — the old guard (max_name_width > 1) silently skipped
truncation entirely, rendering the full name and defeating the whole
point of the feature. Now we just show "…" when there's no room.

Add unit tests for digit_count because it's a pure function and
there's no excuse not to.

Also add .DS_Store to .gitignore because apparently that wasn't
done yet. Please don't commit macOS metadata files.
…lication

The previous truncation logic used chars().count() to measure path
widths, which is *wrong* for East Asian characters — a CJK char takes
2 display columns but counts as 1 char. Narrow your terminal enough
with a Japanese directory name and the stats still get pushed off
screen. Not great.

Switch to unicode-width (already a transitive dep via ratatui, so
this costs us exactly zero new crate downloads) for proper display
column measurement.

While at it, the stats width calculation and the stats span building
were encoding the same format in two separate places — a classic
"change one, forget the other" bug waiting to happen. Extract both
into build_file_stats() which returns the spans *and* their width in
one pass. The now-orphaned digit_count() goes away with it.

Also replace the magic `4` with a named constant because unnamed
magic numbers are how you end up debugging layout issues at 2am.
The previous commit switched truncate_path to unicode-width for
correct display column measurement, but build_file_stats was still
using s.len() — which is byte length, not display width. For the
ASCII-only stats strings we produce today that's technically fine,
but it's the kind of inconsistency that bites you later when someone
changes the format and doesn't realize half the width math uses one
measurement system and half uses another.

Use UnicodeWidthStr::width() consistently. While at it, add unit
tests for build_file_stats covering the basic cases (adds-only,
dels-only, both, neither, large numbers) with a round-trip assertion
that the returned width actually matches the rendered span width.
The kind of test that makes future divergence impossible to miss.
@bahdotsh
bahdotsh merged commit 9523da7 into main Mar 20, 2026
5 checks passed
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.

1 participant