From 4a41dbd7473911b0bb078fa0cbfa5a44811e7f56 Mon Sep 17 00:00:00 2001 From: l0ng-ai <24760907+l0ng-ai@users.noreply.github.com> Date: Sun, 16 Aug 2026 00:46:02 +0800 Subject: [PATCH] fix(ui): cut display text between clusters, not inside them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `tab_strip::clusters` already writes down why a label may only be cut on grapheme boundaries: `👨‍👩‍👧` loses the joiner holding it together, `❤️` loses the variation selector that makes it an emoji, and `🇨🇳` leaves a lone regional indicator that renders as a bare letter. Three other functions cut display text by `char` and so do exactly that. `elide_middle` is the one that shows the gap most clearly. It documents its cuts as safe because "everything here walks `chars()`, never bytes" — true, and it guards the hazard that produces invalid UTF-8, which is not the hazard that reaches the screen. Its sibling test is named `never_cuts_a_multibyte_char_in_half`; the cluster is the unit above that one, and nothing was holding it. Each function keeps its own algorithm — head cut, middle cut, ellipsis budget — and only the unit it counts in changes. That also makes the budgets more honest than they were: a budget in clusters is a budget in what actually gets drawn, where a budget in `char`s let one flag spend two of it. Callers pass ASCII in every existing test, so no rendered string that was already correct changes. --- src/terminal/view.rs | 23 +++++++++++++++++++++-- src/ui/home.rs | 32 ++++++++++++++++++++++++++++++-- src/ui/scm/path.rs | 42 ++++++++++++++++++++++++++++++++++-------- 3 files changed, 85 insertions(+), 12 deletions(-) diff --git a/src/terminal/view.rs b/src/terminal/view.rs index 19208db8..ec72d4d3 100644 --- a/src/terminal/view.rs +++ b/src/terminal/view.rs @@ -32,6 +32,7 @@ use crate::core::actions::{ use crate::core::config::{BellMode, Config, LinkFileOpen, NotifyMode}; use crate::daemon::protocol::{RemoteContext, ShellSpec}; use crate::ui::i18n::{L10nKey, t, t_fmt}; +use unicode_segmentation::UnicodeSegmentation as _; const GRID_PAD_X: f32 = 8.; const GRID_PAD_Y: f32 = 4.; @@ -6552,14 +6553,20 @@ fn description_budget(cell_width: f32, label_cells: usize, menu_w: f32) -> usize /// `text` cut to `budget` characters, with the last one spent on an ellipsis. /// A budget too small to say anything with returns nothing rather than a bare /// "…", which reads as a description that is there but unreadable. +/// +/// "Character" is a grapheme cluster, the unit `clusters` in the tab strip +/// explains: a completion description is arbitrary text from a shell plugin, +/// and cutting a cluster in half renders as a character the description never +/// contained. fn elide(text: &str, budget: usize) -> String { - if text.chars().count() <= budget { + let clusters: Vec<&str> = text.graphemes(true).collect(); + if clusters.len() <= budget { return text.to_string(); } if budget < 2 { return String::new(); } - let mut out: String = text.chars().take(budget - 1).collect(); + let mut out: String = clusters[..budget - 1].concat(); out.push('…'); out } @@ -8045,6 +8052,18 @@ mod tests { assert_eq!(elide("abcd", 4), "abcd"); } + #[test] + fn an_elided_description_spends_its_budget_on_whole_clusters() { + use unicode_segmentation::UnicodeSegmentation as _; + // A shell plugin's description is arbitrary text, emoji included. + let out = elide("🇨🇳🇨🇳🇨🇳🇨🇳🇨🇳 open the thing", 4); + assert_eq!(out.graphemes(true).count(), 4, "{out:?}"); + assert_eq!(out, "🇨🇳🇨🇳🇨🇳…"); + // The budget is in clusters, so it holds three flags and an ellipsis + // rather than one and a half flags reading as "CN CN C". + assert!(out.ends_with('…'), "{out:?}"); + } + #[test] fn an_overlong_description_ends_in_an_ellipsis_inside_its_budget() { let out = elide("Move or rename a file, a directory, or a symlink", 12); diff --git a/src/ui/home.rs b/src/ui/home.rs index 6c3b5def..8ac73519 100644 --- a/src/ui/home.rs +++ b/src/ui/home.rs @@ -7,6 +7,7 @@ use gpui::{ use gpui_component::button::{Button, ButtonVariants as _}; use gpui_component::kbd::Kbd; use gpui_component::{ActiveTheme as _, IconName, Sizable as _, h_flex, v_flex}; +use unicode_segmentation::UnicodeSegmentation as _; use crate::core::session::{SessionPane, SessionTab}; use crate::ui::app::Tty7App; @@ -55,9 +56,17 @@ fn first_leaf_cwd(pane: &SessionPane) -> Option<&std::path::PathBuf> { } } +/// `s` cut to [`CLOSED_LABEL_MAX`] clusters, with an ellipsis when it was cut. +/// +/// Counted in grapheme clusters rather than `char`s for the reason +/// `tab_strip::clusters` gives: a directory name is a name a person chose, +/// and cutting `❤️` or `🇨🇳` by `char` drops the variation selector or one +/// half of the flag, so the cut renders as a different character than the +/// one it kept. fn clamp_label(s: &str) -> String { - if s.chars().count() > CLOSED_LABEL_MAX { - format!("{}…", s.chars().take(CLOSED_LABEL_MAX).collect::()) + let clusters: Vec<&str> = s.graphemes(true).collect(); + if clusters.len() > CLOSED_LABEL_MAX { + format!("{}…", clusters[..CLOSED_LABEL_MAX].concat()) } else { s.to_string() } @@ -396,6 +405,25 @@ mod tests { assert_eq!(closed_tab_label(&root), None); } + #[test] + fn closed_tab_label_never_cuts_a_cluster_in_half() { + // 24 flags: over the budget, and every one of them two code points + // that mean a letter apiece if they are separated. + let tab = SessionTab { + name: Some("🇨🇳".repeat(24)), + tree_id: None, + sidebar_group: None, + pane: leaf(None), + }; + let label = closed_tab_label(&tab).unwrap(); + assert_eq!(label, format!("{}…", "🇨🇳".repeat(CLOSED_LABEL_MAX))); + // Counting `char`s would have taken CLOSED_LABEL_MAX code points — + // half as many flags, the last of them split into a bare regional + // indicator that draws as the letter C. + assert_eq!(label.graphemes(true).count(), CLOSED_LABEL_MAX + 1); + assert_eq!(label.chars().count(), CLOSED_LABEL_MAX * 2 + 1); + } + #[test] fn closed_tab_label_clamps_runaway_names() { let tab = SessionTab { diff --git a/src/ui/scm/path.rs b/src/ui/scm/path.rs index c8485d46..542a1cbb 100644 --- a/src/ui/scm/path.rs +++ b/src/ui/scm/path.rs @@ -3,8 +3,9 @@ //! once per visible row per frame. use std::borrow::Cow; +use unicode_segmentation::UnicodeSegmentation as _; -/// The ellipsis every eliding function here uses. One `char`, so a budget in +/// The ellipsis every eliding function here uses. One cluster, so a budget in /// characters is a budget the caller can reason about. const ELLIPSIS: char = '…'; @@ -27,18 +28,26 @@ pub(crate) fn split_display_path(rel: &str) -> (&str, &str) { /// carry their meaning at the ends — `feature/…/auth-retry` still says which /// area and which change, where a plain truncate says neither. /// -/// `max_chars` counts characters including the ellipsis, so the result never -/// renders wider than the caller budgeted. Cuts land on character boundaries by -/// construction: everything here walks `chars()`, never bytes. +/// `max_chars` counts what a reader counts as one character — grapheme +/// clusters — including the ellipsis, so the result never renders wider than +/// the caller budgeted. +/// +/// Walking `chars()` would already put every cut on a `char` boundary and so +/// never produce invalid UTF-8, which is the hazard that usually gets the +/// attention. It is not the hazard that shows: a filename holding `❤️` or +/// `🇨🇳` cut mid-cluster loses the variation selector or one regional +/// indicator, and the row then draws a character the path does not contain. +/// `tab_strip::clusters` states the same rule for tab labels. pub(crate) fn elide_middle(s: &str, max_chars: usize) -> Cow<'_, str> { - let total = s.chars().count(); + let clusters: Vec<&str> = s.graphemes(true).collect(); + let total = clusters.len(); if total <= max_chars { return Cow::Borrowed(s); } // Below three there is no room for head + ellipsis + tail; fall back to a // plain head cut rather than returning something wider than asked for. if max_chars <= 2 { - return Cow::Owned(s.chars().take(max_chars).collect()); + return Cow::Owned(clusters[..max_chars].concat()); } let keep = max_chars - 1; // Bias the extra character to the head: the tail is usually a file name, @@ -46,9 +55,9 @@ pub(crate) fn elide_middle(s: &str, max_chars: usize) -> Cow<'_, str> { let head = keep.div_ceil(2); let tail = keep - head; let mut out = String::with_capacity(s.len()); - out.extend(s.chars().take(head)); + out.push_str(&clusters[..head].concat()); out.push(ELLIPSIS); - out.extend(s.chars().skip(total - tail)); + out.push_str(&clusters[total - tail..].concat()); Cow::Owned(out) } @@ -111,6 +120,23 @@ mod tests { assert!(matches!(elide_middle("exact", 5), Cow::Borrowed("exact"))); } + #[test] + fn elide_middle_cuts_between_clusters_not_inside_them() { + // Both ends are clusters that mean something else in pieces: a family + // is four code points joined by ZWJ, a flag is two indicators. + let s = "👨‍👩‍👧/aaaaaaaaaaaaaaaaaaaa/🇨🇳"; + let out = elide_middle(s, 8); + assert_eq!(out.graphemes(true).count(), 8, "{out:?}"); + assert!(out.starts_with("👨‍👩‍👧"), "{out:?}"); + assert!(out.ends_with("🇨🇳"), "{out:?}"); + // Cutting by `char` would have spent four of its seven on the family + // alone and split it, so the head would have ended in a dangling ZWJ + // that then attaches itself to the ellipsis. + let by_char: String = s.chars().take(4).collect(); + assert!(by_char.ends_with('\u{200D}'), "{by_char:?}"); + assert!(!out.contains("👨\u{200D}👩\u{200D}…"), "{out:?}"); + } + #[test] fn elide_middle_keeps_both_ends_and_respects_the_budget() { let out = elide_middle("crates/tty7-core/src/core/git/status.rs", 20);