Merge pull request #145 from fiddlerwoaroof/fix/t14d-title-bar-escape-hatch
fix(tui): the title bar's escape hatch outranks the document name (improvise-t14d)
This commit is contained in:
@@ -154,6 +154,35 @@ composes both: the prompt and buffer on the left, any pending `status_msg`
|
||||
right-aligned. The alternatives and why they lost are in
|
||||
`context/test-evidence/tui.md`.
|
||||
|
||||
### `fill_line` — which segment yields is a per-call-site decision
|
||||
|
||||
Three rows in the TUI are one left segment and one right segment sharing a line:
|
||||
the title bar, `minibuffer_line` and `draw_status`. `fill_line` composes them,
|
||||
and it owes two things that it did not originally do — it must measure in
|
||||
**display cells** (it padded with `left.len()`, bytes, and the title's own `·`
|
||||
is two of them, so every document's right segment sat one cell in from its edge)
|
||||
and it must **truncate**, because concatenating and letting ratatui clip clips
|
||||
the *right*.
|
||||
|
||||
What clipping the right costs depends on the row, which is why `Keeps` is a
|
||||
parameter and not a rule:
|
||||
|
||||
- **The title bar keeps the right.** Its left segment is decoration whose length
|
||||
is unbounded — a document name plus a filename — and its right segment is the
|
||||
fixed `" ?:help :q quit "`, the only thing on screen telling a stuck user how
|
||||
to get out. Unconditional left priority made the escape hatch the first thing
|
||||
sacrificed, at 100 columns with an ordinary document name (improvise-t14d).
|
||||
- **Both bottom-bar rows keep the left**, which is what they already did. There
|
||||
the left is the user's own typing plus the cursor (`minibuffer_line`) or the
|
||||
status message (`draw_status`), and the right is a view badge or the message's
|
||||
own margin. Right priority there would let a long message eat the text being
|
||||
edited — the same mistake in the other direction.
|
||||
|
||||
So the rule is not "the right wins" but "the affordance wins over the
|
||||
decoration", and which is which is local. Truncation goes through
|
||||
`ui::text::truncate` (improvise-8l0c), so an overflow is a `…` rather than a
|
||||
mid-token clip of `?:`.
|
||||
|
||||
### The caches are built here, per frame
|
||||
|
||||
`project::formulas_of`, `category_tree_of`, `view_list_of`, `tile_bar_of`,
|
||||
|
||||
@@ -644,3 +644,89 @@ Fault injected 2026-09-21: restoring the name-only comparison in
|
||||
`AxisFrame::group_runs` fails this with `left: [(Some("Q1"), 4)]` while the five
|
||||
existing agreement tests all pass — the ungrouped `Sep` in their fixture is
|
||||
precisely why they cannot see it.
|
||||
|
||||
---
|
||||
|
||||
## `src/draw.rs::title_bar_tests` — the title bar's escape hatch
|
||||
|
||||
improvise-t14d. `fill_line` composed a left and a right segment into one row and
|
||||
did **neither** of the two things that composition needs. It padded with
|
||||
`left.len() + right.len()` — **bytes** — and it never truncated, so on overflow
|
||||
it simply concatenated and left ratatui to clip. ratatui clips the *right*, and
|
||||
the right segment of the title bar is `" ?:help :q quit "`: the only thing on
|
||||
screen telling a stuck user how to get help or get out.
|
||||
|
||||
**The priority was backwards.** The left segment is decoration — the product
|
||||
name, the document name, the filename, a dirty marker — and its length is
|
||||
unbounded because a document name is. The right segment is a fixed 17 cells and
|
||||
is the escape hatch. Giving the unbounded decoration unconditional priority over
|
||||
the fixed affordance means the affordance is the first thing sacrificed, and it
|
||||
is sacrificed at ordinary widths: measured through a real `TestBackend`, a
|
||||
document named `Quarterly Regional Sales Analysis` stored as
|
||||
`quarterly-regional-sales-2026.improv` loses `:q quit` **at 100 columns**.
|
||||
|
||||
The byte fault is separate and universal. The title's own format string spells
|
||||
`·`, two bytes and one cell, so `left.len()` over-counts by one for *every*
|
||||
document — the padding is one column short and the right segment sits one cell
|
||||
in from the edge it is anchored to, before any non-ASCII document name makes it
|
||||
worse.
|
||||
|
||||
`Keeps` is the fix's shape: which segment survives is a per-call-site decision,
|
||||
not a global rule. The title bar keeps the **right** (the hatch outranks the
|
||||
name). The two bottom-bar call sites keep the **left**, which is what they did
|
||||
before — there the left is the user's own typing (`minibuffer_line`) or the
|
||||
status message (`draw_status`), and the right is a view badge, so right-priority
|
||||
would have let a long message eat the text the user is editing along with the
|
||||
cursor. Widths are `ui::text::truncate`'s display cells throughout, so the
|
||||
overflow is a graceful `…` rather than a mid-token clip of `?:`.
|
||||
|
||||
### the_escape_hatch_outlives_a_long_document_name
|
||||
|
||||
The defect itself, at the five widths the ticket measured (36, 44, 52, 60, 100).
|
||||
Rendered through a `TestBackend` and read off the terminal buffer, not off
|
||||
`fill_line`'s return value: the loss happens in ratatui's clip, so a helper's
|
||||
return value cannot see it. **A test at 80 columns with a short fixture name
|
||||
passes against the broken code** — title length is the variable, not width.
|
||||
|
||||
Fault injected: `Keeps::Right` → `Keeps::Left` at the `draw_title` call site.
|
||||
Fails at width 36 with `" improvise · Quarterly Regional S"`; the other four
|
||||
tests in the module pass, so this one and only this one names the priority.
|
||||
|
||||
### the_document_name_still_leads_the_row
|
||||
|
||||
The other direction, which the escape-hatch assertion alone cannot supply: a fix
|
||||
that dropped the left segment entirely, or that put the hatch first, satisfies
|
||||
it. At 80 columns with a short name nothing is truncated, so this pins that the
|
||||
name leads, the hatch follows, and the filename survives intact.
|
||||
|
||||
### a_multibyte_title_does_not_shift_the_right_segment
|
||||
|
||||
The byte-versus-cell fault. Asserts the row *ends with* the right segment — the
|
||||
edge it is anchored to — for an ASCII name, an accented one and a CJK one. The
|
||||
ASCII case is the discriminating one and the surprise: it fails against the
|
||||
broken code too, because the `·` is in the format string rather than in the
|
||||
document's name.
|
||||
|
||||
Fault injected: `width - left.width() - right.width()` →
|
||||
`width.saturating_sub(left.len() + right.len())`, keeping the truncation. Fails
|
||||
on `"Sales"` with the right segment one cell in from the edge, and takes
|
||||
`a_composed_line_is_exactly_its_width` with it. The escape-hatch test does not
|
||||
see it, which is why the two faults need two tests.
|
||||
|
||||
### a_composed_line_is_exactly_its_width
|
||||
|
||||
The property both directions owe, over 0..40 columns, three segment pairs and
|
||||
both `Keeps` arms: a wide-character left segment, an empty left, and an empty
|
||||
right. Width 0 and the band where the two segments cannot both fit are the cases
|
||||
`fill_line`'s arithmetic can get wrong — it subtracts rather than saturating, so
|
||||
an off-by-one here is a panic and not a cosmetic slip.
|
||||
|
||||
### the_kept_side_is_the_side_that_survives
|
||||
|
||||
`Keeps::Left` has no other witness in this module: every rendering test above
|
||||
drives the title bar, which keeps the right. Without this, both arms could be
|
||||
the same code and the suite would stay green — which is what the enum exists to
|
||||
prevent.
|
||||
|
||||
Fault injected: the `Keeps::Left` arm rewritten to the body of the `Keeps::Right`
|
||||
arm. This test fails; the other four pass.
|
||||
|
||||
+140
-5
@@ -24,11 +24,13 @@ use crate::ui::grid::GridWidget;
|
||||
use crate::ui::help::HelpWidget;
|
||||
use crate::ui::import_wizard_ui::ImportWizardWidget;
|
||||
use crate::ui::panel::Panel;
|
||||
use crate::ui::text::truncate;
|
||||
use crate::ui::tile_bar::TileBar;
|
||||
use crate::ui::view_panel::ViewContent;
|
||||
use crate::ui::which_key::WhichKeyWidget;
|
||||
use crate::view::AxisColor;
|
||||
use improvise_command::ui::project;
|
||||
use unicode_width::UnicodeWidthStr;
|
||||
|
||||
struct TuiContext<'a> {
|
||||
terminal: Terminal<CrosstermBackend<&'a mut Stdout>>,
|
||||
@@ -169,8 +171,29 @@ fn minibuf_fg(color: MinibufColor) -> Color {
|
||||
}
|
||||
}
|
||||
|
||||
fn fill_line(left: String, right: &str, width: u16) -> String {
|
||||
let pad = " ".repeat((width as usize).saturating_sub(left.len() + right.len()));
|
||||
/// Which segment survives when the two cannot both fit. The title bar's escape
|
||||
/// hatch outranks the document name (improvise-t14d) — `context/code/tui.md`.
|
||||
#[derive(Clone, Copy, Debug)]
|
||||
enum Keeps {
|
||||
Left,
|
||||
Right,
|
||||
}
|
||||
|
||||
fn fill_line(left: String, right: &str, width: u16, keeps: Keeps) -> String {
|
||||
let width = width as usize;
|
||||
let (left, right) = match keeps {
|
||||
Keeps::Right => {
|
||||
let right = truncate(right, width);
|
||||
let left = truncate(&left, width - right.width());
|
||||
(left, right)
|
||||
}
|
||||
Keeps::Left => {
|
||||
let left = truncate(&left, width);
|
||||
let right = truncate(right, width - left.width());
|
||||
(left, right)
|
||||
}
|
||||
};
|
||||
let pad = " ".repeat(width - left.width() - right.width());
|
||||
format!("{left}{pad}{right}")
|
||||
}
|
||||
|
||||
@@ -272,7 +295,7 @@ fn draw_title(f: &mut Frame, area: Rect, app: &App) {
|
||||
let file = doc.stored_as.map(|n| format!(" ({n})")).unwrap_or_default();
|
||||
let title = format!(" improvise · {}{}{} ", doc.name, file, dirty);
|
||||
let right = " ?:help :q quit ";
|
||||
let line = fill_line(title, right, area.width);
|
||||
let line = fill_line(title, right, area.width, Keeps::Right);
|
||||
f.render_widget(
|
||||
Paragraph::new(line).style(
|
||||
Style::default()
|
||||
@@ -396,7 +419,7 @@ fn minibuffer_line(prompt: &str, buf: &str, status_msg: &str, width: u16) -> Str
|
||||
true => String::new(),
|
||||
false => format!(" {status_msg} "),
|
||||
};
|
||||
fill_line(left, &right, width)
|
||||
fill_line(left, &right, width, Keeps::Left)
|
||||
}
|
||||
|
||||
fn draw_bottom_bar(f: &mut Frame, area: Rect, app: &App) {
|
||||
@@ -445,7 +468,7 @@ fn draw_status(f: &mut Frame, area: Rect, app: &App) {
|
||||
" {}{search_part} {msg}",
|
||||
crate::protocol::mode_label(&app.view_state.mode)
|
||||
);
|
||||
let line = fill_line(left, &view_badge, area.width);
|
||||
let line = fill_line(left, &view_badge, area.width, Keeps::Left);
|
||||
|
||||
f.render_widget(
|
||||
Paragraph::new(line).style(mode_style(&app.view_state.mode)),
|
||||
@@ -687,3 +710,115 @@ mod key_conversion_tests {
|
||||
assert_eq!(got, key::KeyModifiers::ALT);
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod title_bar_tests {
|
||||
use super::{Keeps, draw_title, fill_line};
|
||||
use crate::ui::app::App;
|
||||
use crate::workbook::Workbook;
|
||||
use ratatui::{Terminal, backend::TestBackend, layout::Rect};
|
||||
use std::path::PathBuf;
|
||||
use unicode_width::UnicodeWidthStr;
|
||||
|
||||
/// The title row as the terminal really receives it: ratatui clips the
|
||||
/// right, so a helper's return value cannot see the defect.
|
||||
fn title_row(app: &App, width: u16) -> String {
|
||||
let mut terminal = Terminal::new(TestBackend::new(width, 1)).expect("test backend");
|
||||
terminal
|
||||
.draw(|f| draw_title(f, Rect::new(0, 0, width, 1), app))
|
||||
.expect("draw");
|
||||
terminal
|
||||
.backend()
|
||||
.buffer()
|
||||
.content()
|
||||
.iter()
|
||||
.map(|c| c.symbol())
|
||||
.collect()
|
||||
}
|
||||
|
||||
fn app_named(name: &str, file: &str) -> App {
|
||||
App::new(Workbook::new(name), Some(PathBuf::from(file)))
|
||||
}
|
||||
|
||||
/// improvise-t14d — `context/test-evidence/tui.md`.
|
||||
#[test]
|
||||
fn the_escape_hatch_outlives_a_long_document_name() {
|
||||
let app = app_named(
|
||||
"Quarterly Regional Sales Analysis",
|
||||
"quarterly-regional-sales-2026.improv",
|
||||
);
|
||||
for width in [36u16, 44, 52, 60, 100] {
|
||||
let row = title_row(&app, width);
|
||||
assert!(
|
||||
row.contains("?:help") && row.contains(":q quit"),
|
||||
"the escape hatch is gone at width {width}: {row:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// The name must still lead where it fits, or a fix that simply dropped it
|
||||
/// would satisfy the assertion above. improvise-t14d.
|
||||
#[test]
|
||||
fn the_document_name_still_leads_the_row() {
|
||||
let app = app_named("Sales", "sales.improv");
|
||||
let row = title_row(&app, 80);
|
||||
let name_at = row.find("Sales").expect("the name is drawn");
|
||||
let hatch_at = row.find("?:help").expect("the hatch is drawn");
|
||||
assert!(
|
||||
name_at < hatch_at,
|
||||
"the name leads and the hatch follows: {row:?}"
|
||||
);
|
||||
assert!(
|
||||
row.contains("(sales.improv)"),
|
||||
"a name that fits is not truncated: {row:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// Display cells, not bytes — the ASCII case is the discriminating one.
|
||||
/// improvise-t14d, `context/test-evidence/tui.md`.
|
||||
#[test]
|
||||
fn a_multibyte_title_does_not_shift_the_right_segment() {
|
||||
for name in ["Sales", "Ventes régionales", "日本語"] {
|
||||
let app = app_named(name, "m.improv");
|
||||
let row = title_row(&app, 80);
|
||||
assert!(
|
||||
row.ends_with(":q quit "),
|
||||
"the right segment must sit on the right edge for {name:?}: {row:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// Whichever segment yields, the composed line is exactly the width it was
|
||||
/// given — the property both call-site directions owe. improvise-t14d.
|
||||
#[test]
|
||||
fn a_composed_line_is_exactly_its_width() {
|
||||
for width in 0u16..40 {
|
||||
for (left, right) in [
|
||||
(" improvise · Ünïcödé ", " ?:help :q quit "),
|
||||
("", " badge "),
|
||||
("日本語 is a wide left segment", ""),
|
||||
] {
|
||||
for keeps in [Keeps::Left, Keeps::Right] {
|
||||
let line = fill_line(left.to_string(), right, width, keeps);
|
||||
assert_eq!(
|
||||
line.width(),
|
||||
width as usize,
|
||||
"fill_line({left:?}, {right:?}, {width}, {keeps:?}) = {line:?}"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// `Keeps::Left` is the other direction, and nothing else in this module
|
||||
/// separates it from `Keeps::Right`. improvise-t14d.
|
||||
#[test]
|
||||
fn the_kept_side_is_the_side_that_survives() {
|
||||
let (left, right) = ("LEFTLEFTLEFT", "RIGHTRIGHT");
|
||||
assert_eq!(fill_line(left.to_string(), right, 12, Keeps::Left), left);
|
||||
assert!(
|
||||
fill_line(left.to_string(), right, 12, Keeps::Right).ends_with(right),
|
||||
"the right segment is kept whole when it is the one that wins"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user