fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Wizard text is clipped to the panel and measured in cells; the list header says what is shown (review B1, B2, Codex P2 on #304)

Browse Source 
Every wizard write goes through Panel, which clips to the panel and saturates
its height arithmetic, so no step panics on a small terminal. The bottom row
truncates in cells, so a wrapped message with a multibyte character is drawn
whole. A scrolling list's range is named on its header row, where no item can
cover it. Tests: a multibyte message at every width, wrap bytes vs cells, a
no-panic sweep of every step at 20-100 x 3-20, the first item filling the list.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
Edward Langley
2026-09-24 19:11:42 -07:00
parent f00ed3e979
commit 22cbfa679e
7 changed files with 510 additions and 149 deletions
Download Patch File Download Diff File Expand all files Collapse all files
context/code/formula-and-io.md
+17 -10
View File
@@ -680,15 +680,20 @@ record lands is `Plan::place`: its categories, then its date parts, then its
labels, then its measure values, or a reason it is dropped. `collisions()` and
`build_model_with_warnings` both call it, so the question the wizard asks and
the thing the build does cannot disagree. Neither has a second walk of the
record. A collision is a tuple reached by **two or more records that each
carry a measure value**. A record with no measure makes no cell, so it cannot
collide. The tuple includes label values, as `CellKey` always has.
record. **A collision is a cell that more than one record writes**, meaning a
tuple plus a measure (improvise-9bit). A `Collision` names the tuple and only
the records that share one of its cells. A record with no measure makes no
cell, so it collides with nothing. The tuple includes label values, as
`CellKey` always has.
Two records on one tuple with **disjoint** measures (one has `Revenue`, the
other `Cost`) count as a collision even though summing them loses nothing. The
unit is the record, the user's own words were "rows collide on every
coordinate", and a cell-level rule would make "which records collided" depend
on which measure you look at.
This PR's first version keyed on the tuple alone. It was **superseded in review
(improvise-9bit)**, and the argument it gave is withdrawn. Under that rule, two
records with **disjoint** measures on one tuple (one has `Revenue`, the other
`Cost`) counted as colliding although no cell sums. The wizard then offered
"Sum them into one cell" where nothing would sum, and with the `Refuse` default
the CLI refused a sparse-measure CSV that had always imported cleanly. The
question the user is asked is "these values would be added together", so the
rule has to be the one that decides whether anything is added.
The data is structured. A `Collision` holds `coords` plus 1-based `records`
positions. `WizardCache::collisions` carries it, and
@@ -710,8 +715,10 @@ at `Refuse`, which `validation_error` blocks, so a bare advance could never
leave the step.
**Separate is an occurrence index, not a row number.** This is the user's
preferred shape. Records on a colliding tuple get `1, 2, 3…` in source order,
and every other record gets `1`. The category holds only as many items as the
preferred shape. The records in a collision get `1, 2, 3…` in source order,
and every other record gets `1`. That includes a record on the same tuple that
shares no cell with the others: it lands on occurrence `1` beside the first
colliding record, and since the two write different measures, nothing sums. The category holds only as many items as the
deepest collision, rather than one item per record, and every existing
coordinate keeps its meaning. Items are added `1..=n` in numeric order
(gotcha 18). The category goes on **`Axis::None`**. Left to
context/code/tui.md
+20 -4
View File
@@ -569,10 +569,14 @@ one-line message therefore draws exactly as before, which is why the pinned
frames (`every_step_draws_what_it_drew_before`,
`the_formula_step_draws_its_transient_state`) did not move.
**Lists scroll by item, and say what they hide.** `draw_scrolling` draws the
items from `WizardCache::scroll` on, each one wrapped whole. It writes a
`▲ n more above` or `▼ n more below` row instead of cutting an item in half.
`ImportWizard::scroll` clamps to `len - 1`, so the last item can reach the top
**Lists scroll by item, and the header says what is shown.** `Panel::scrolling`
draws the items from `WizardCache::scroll` on, each one wrapped whole. The
list's own header row reads `Colliding records 3–9 of 14 (PgUp/PgDn):` whenever
some items are off screen. A first version put "more above" and "more below"
markers inside the list's rows. When the first item filled the list exactly,
the marker landed on the hint row and was drawn over (Codex, PR #304). The
header row belongs to no item, so no item can cover it, and no item loses a line
to make room for a marker. `ImportWizard::scroll` clamps to `len - 1`, so the last item can reach the top
and the list can never scroll away. It is by item, not by line, because the
wizard state cannot know the panel width that line-level clamping needs. The
residual is honest. A **single** item taller than the whole list area, roughly
@@ -580,5 +584,17 @@ residual is honest. A **single** item taller than the whole list area, roughly
wizard warning is that long today: a collision tuple is bounded by twelve
categories plus labels, but a label value is not bounded.
**Every write is clipped to the panel.** The widget draws through `Panel`,
whose `put` writes only inside the panel's inner rectangle, and its height
arithmetic saturates. Before this change, the duplicates step wrote at an
unbounded row and ratatui panicked at 80×10, and four older steps panicked on
`u16` underflow at heights of 3 to 5 (review B2 on PR #304). The bottom row's
`set_bottom_line` now truncates in cells through `ui::text::truncate`. It had
used the widget's byte-measured `truncate`, so a wrapped line holding a `←`
was cut one character early, with `…`, although it fit (review B1). The local
byte `truncate` still draws field names and labels. It is byte-identical on
ASCII, which keeps every pinned frame, and those rows are labels rather than
messages.
Other TUI surfaces still truncate text a user needs. They are out of this
ticket's scope and reported in its hand-back rather than fixed here.
context/test-evidence/formula-and-io.md
+42 -5
View File
@@ -1097,12 +1097,15 @@ loop was therefore re-ordered to `[backward, forward]` under the sort fault and
observed failing on the backward history specifically, with
`left: ["alpha", …]` (sorted) against `right: ["charlie", …]` (backward).
### a_long_collision_list_is_truncated_with_a_count
### every_summed_cell_is_reported_in_full
The warning is rendered into a one-line status bar. A file with ten thousand
duplicate coordinates would otherwise compose a warning naming every one. Three
cells are named and the rest counted; the empty case returns `None` rather than
a warning that says nothing happened.
This replaces `a_long_collision_list_is_truncated_with_a_count`, which pinned
2cw9's three-then-"and N more" warning. After the Sum choice, the Confirm step
still showed "…; and 28 more". The user's rule is that a message they need is
never truncated, so each summed cell is now its own warning, and the step wraps
and scrolls them. It was red at compile time against the old `Option<String>`
signature, then red against a `Vec` that still held one elided sentence. The
rival **elided after three** fails it.
### build_model_warns_about_records_skipped_for_missing_category
@@ -1577,3 +1580,37 @@ Also `choosing_go_back_returns_to_the_step_before`, `back_retraces_the_duplicate
option" are different. The rival **chooser ignores the cursor** fails six of
them. **suggestion ignores headers** and **name check accepts a taken name**
each fail their own test.
### records_with_disjoint_measures_on_one_tuple_do_not_collide
improvise-9bit, found by the adversarial review of PR #304 at `92233cff`.
`collisions_among` keyed on the tuple alone, so `{East, revenue}` and
`{East, cost}` were reported as colliding although no cell sums. Under the
`Refuse` default, a sparse-measure CSV that imported cleanly before was refused.
It was red before the fix: `collisions()` returned the East group. The rival
**tuple-only rule** fails it, and fails
`only_the_records_that_share_a_cell_are_listed_and_numbered` too.
### a_record_with_no_measure_collides_with_nothing
The rule the reviewer's injection `if false && measures.is_empty()` showed was
unguarded. It asserts records `[1, 3]` with the measureless `#2` excluded, and
that the two measureless West records form no collision at all. It was green
before the fix, since the rule already held, so its evidence is the rival:
**a measureless record counts as writing** fails it.
### only_the_records_that_share_a_cell_are_listed_and_numbered
A tuple with one `cost` record and two `revenue` records. Only `#2` and `#3`
are listed. Under Separate, the `cost` record keeps occurrence `1`, beside the
first `revenue` record, and the second `revenue` record gets `2`. The rivals
**tuple-only rule** and **a non-colliding record numbered past 1** each fail it.
### the_name_check_covers_categories_derived_from_dates
improvise-4klp, filed by the same review. The name step's check against
date-derived categories was untested. The test extracts `when_Year` and types
that name, which must be refused. It was green on arrival, so its evidence is
the reviewer's rival, **date-derived names dropped from the taken names**,
which fails it. That fault would have let `add_category` reuse the existing
category, and every cell key would then name `when_Year` twice.
context/test-evidence/tui.md
+73 -3
View File
@@ -1354,8 +1354,78 @@ Its rival **message drawn on one line** fails it.
### scrolling_reaches_the_last_collision
Fourteen colliding tuples, more than fit. The first frame must show the first
tuple and a `more below` marker, and not show the last one. After scrolling,
the last tuple and a `more above` marker must be drawn. The rivals **list
ignores the scroll offset** and **no marker for what lies below** each fail it.
tuple and a header naming the range shown, and not show the last one. After scrolling,
the last tuple must be drawn. Since the Codex fix, the header row names the
range shown (`Colliding records 14–14 of 14`) in place of the in-list markers.
The rivals **list ignores the scroll offset** and **the header claims every item
shown** each fail it.
`keeping_separate_at_the_cap_is_drawn_unavailable_with_its_reason` fails under
**disabled reason not drawn**.
### a_multibyte_message_is_drawn_whole_at_every_width
Review B1 on PR #304. Messages were wrapped in cells and then drawn through
`set_bottom_line`, which truncated with the widget's **byte**-measured
`truncate`. A wrapped line holding a `←` (three bytes) that fit in cells was
still cut, with `…`. At 80 columns the category-cap reason lost `(←)`, and
words were lost at 22 panel widths from 30 to 100. The earlier
`a_message_longer_than_the_panel_is_drawn_whole` was ASCII-only and could not
see it. This test drives the real cap reason and a message of accents, CJK
and emoji across every width from 30 to 100. It asserts no `…` and every word
in order. It was red before the fix. The rival **bottom row measured in bytes
again** fails it.
Its first run exposed a bug in the test's own helper. `buf_text` read the
padding cell after a double-width character as a space, so `日本語` read as
`日 本 語`. The helper now skips that cell. This changes nothing for the ASCII
pinned frames.
### wrapping_measures_cells_not_bytes
Review B1's second half. A byte count is never smaller than a cell count, so
byte-wrapping only ever wraps early, and "no line wider than asked" plus "no
word lost" cannot tell the two apart. The reviewer swapped `.width()` for
`.len()` in `wrap_words` and the lib stayed green. This test takes strings that
fit in cells but not in bytes (`←←← éé` is 6 cells and 14 bytes; `日本 ab` is 7
cells and 10 bytes) and asserts each stays on **one** line. That rival now
fails it. It was green on arrival, since `wrap` was already right.
### no_step_panics_on_a_small_terminal
Review B2 on PR #304. The duplicates step wrote at an unbounded row, and
ratatui panicked with index-outside-buffer at 80×≤10, 60×≤12 and 50×≤14. It
sweeps every step the wizard can reach at every size from 20 to 100 wide and
3 to 20 high:
- Preview, SelectArrayPath, ReviewProposals and ConfigureDates;
- ResolveDuplicates, both plain and at the cap with its message;
- NameDuplicateCategory;
- DefineFormulas, empty and with formulas and an open editor;
- NameModel, and Done with and without warnings.
It was red before the fix with 3,513 panics, and not only in the new steps.
NameDuplicateCategory panicked up to 13 high, and ResolveDuplicates at every
height to 20. Preview, SelectArrayPath, DefineFormulas and NameModel panicked
at heights 3 to 5 on `u16` underflow in `inner.height - N`, which predates
this ticket.
The fix is `Panel`, one clipped writer, plus saturating arithmetic. Its rivals
are **writes not clipped to the panel** and **height arithmetic not
saturating**. The second one **survived** at first, because the sweep's
DefineFormulas step had no formulas, so the loop holding `inner.bottom() - 5`
never ran. A DefineFormulas step with formulas was added, and the rival now
fails.
The steps run in parallel threads, each over its own prebuilt `WizardCache`.
Run serially the sweep took 3.8s, over the 2-second binary limit, and 0.55s
for the whole lib in parallel on an Apple M4 Max.
### a_first_item_that_fills_the_list_still_says_more_follow
The Codex P2 on PR #304 at `92233cff`. When the first visible item's wrapped
height exactly filled the list, the "more below" marker was written on the next
row, the hint row, and the hint drew over it. The fix moves the scroll status
to the list's header row, which no item can cover. This test fills three rows
with a three-line item and a second item after it. It asserts the first item
is drawn whole and the header reads `1–1 of 2`. The rival **the header claims
every item shown** fails it.
crates/improvise-io/src/import/wizard.rs
+4 -1
View File
@@ -435,10 +435,13 @@ impl Plan<'_> {
}
}
/// A tuple, and each record on it: its 1-based position and the measures it writes.
type OnTuple<'a> = (&'a Vec<(String, String)>, Vec<(usize, Vec<&'a str>)>);
/// The tuples where two records write the same cell (improvise-9bit), with
/// only the records that do. Disjoint measures on one tuple sum nothing.
fn collisions_among(placements: &[Placement]) -> Vec<Collision> {
let mut tuples: Vec<(&Vec<(String, String)>, Vec<(usize, Vec<&str>)>)> = vec![];
let mut tuples: Vec<OnTuple> = vec![];
let mut index: HashMap<CellKey, usize> = HashMap::new();
for (at, placement) in placements.iter().enumerate() {
let Placement::Placed {
src/ui/import_wizard_ui.rs
+343 -126
View File
@@ -61,6 +61,7 @@ impl<'a> Widget for ImportWizardWidget<'a> {
.title(title);
let inner = block.inner(popup_area);
block.render(popup_area, buf);
let mut panel = Panel { buf, clip: inner };
let mut y = inner.y;
let x = inner.x;
@@ -69,9 +70,9 @@ impl<'a> Widget for ImportWizardWidget<'a> {
match &self.wizard.step {
WizardStepKind::Preview => {
let summary = &self.wizard.preview_summary;
buf.set_string(x, y, truncate(summary, w), Style::default());
panel.put(x, y, truncate(summary, w), Style::default());
y += 2;
buf.set_string(
panel.put(
x,
y,
"Press Enter to continue\u{2026}",
@@ -79,7 +80,7 @@ impl<'a> Widget for ImportWizardWidget<'a> {
);
}
WizardStepKind::SelectArrayPath => {
buf.set_string(
panel.put(
x,
y,
"Select the path containing records:",
@@ -87,7 +88,7 @@ impl<'a> Widget for ImportWizardWidget<'a> {
);
y += 1;
for (i, path) in self.wizard.array_paths.iter().enumerate() {
if y >= inner.y + inner.height {
if y >= inner.bottom() {
break;
}
let is_sel = i == self.wizard.cursor;
@@ -102,11 +103,11 @@ impl<'a> Widget for ImportWizardWidget<'a> {
// The root path is already spelled "(root)" by the
// projection, so both ends say it the same way.
let label = format!(" {path}");
buf.set_string(x, y, truncate(&label, w), style);
panel.put(x, y, truncate(&label, w), style);
y += 1;
}
y += 1;
buf.set_string(
panel.put(
x,
y,
"\u{2191}\u{2193} select Enter confirm \u{2190} back",
@@ -114,7 +115,7 @@ impl<'a> Widget for ImportWizardWidget<'a> {
);
}
WizardStepKind::ReviewProposals => {
buf.set_string(
panel.put(
x,
y,
"Review field proposals (Space toggle, c cycle kind):",
@@ -122,7 +123,7 @@ impl<'a> Widget for ImportWizardWidget<'a> {
);
y += 1;
let header = format!(" {:<20} {:<22} {:<6}", "Field", "Kind", "Accept");
buf.set_string(
panel.put(
x,
y,
truncate(&header, w),
@@ -133,7 +134,7 @@ impl<'a> Widget for ImportWizardWidget<'a> {
y += 1;
for (i, proposal) in self.wizard.proposals.iter().enumerate() {
if y >= inner.y + inner.height - 2 {
if y >= inner.bottom().saturating_sub(2) {
break;
}
let is_sel = i == self.wizard.cursor;
@@ -163,12 +164,11 @@ impl<'a> Widget for ImportWizardWidget<'a> {
Style::default().fg(Color::DarkGray)
};
buf.set_string(x, y, truncate(&row, w), style);
panel.put(x, y, truncate(&row, w), style);
y += 1;
}
let hint_y = inner.y + inner.height - 1;
set_bottom_line(
buf,
let hint_y = inner.bottom().saturating_sub(1);
panel.bottom_line(
x,
hint_y,
w,
@@ -177,7 +177,7 @@ impl<'a> Widget for ImportWizardWidget<'a> {
);
}
WizardStepKind::ConfigureDates => {
buf.set_string(
panel.put(
x,
y,
"Select date components to extract (Space toggle):",
@@ -187,11 +187,11 @@ impl<'a> Widget for ImportWizardWidget<'a> {
let mut item_idx = 0;
for proposal in &self.wizard.date_fields {
if y >= inner.y + inner.height - 2 {
if y >= inner.bottom().saturating_sub(2) {
break;
}
let header = format!(" {} (format: {})", proposal.field, proposal.format);
buf.set_string(
panel.put(
x,
y,
truncate(&header, w),
@@ -204,7 +204,7 @@ impl<'a> Widget for ImportWizardWidget<'a> {
// Components arrive in the wizard's own order, already
// paired with their state, so the cursor counts these rows.
for (label, enabled) in &proposal.components {
if y >= inner.y + inner.height - 2 {
if y >= inner.bottom().saturating_sub(2) {
break;
}
let enabled = *enabled;
@@ -221,14 +221,13 @@ impl<'a> Widget for ImportWizardWidget<'a> {
} else {
Style::default().fg(Color::DarkGray)
};
buf.set_string(x, y, truncate(&row, w), style);
panel.put(x, y, truncate(&row, w), style);
y += 1;
item_idx += 1;
}
}
let hint_y = inner.y + inner.height - 1;
set_bottom_line(
buf,
let hint_y = inner.bottom().saturating_sub(1);
panel.bottom_line(
x,
hint_y,
w,
@@ -244,7 +243,7 @@ impl<'a> Widget for ImportWizardWidget<'a> {
What should the import do with them?",
wiz.collisions.len()
);
y = draw_wrapped(buf, x, y, w, &intro, Style::default().fg(Color::Yellow));
y = panel.wrapped(x, y, w, &intro, Style::default().fg(Color::Yellow));
y += 1;
for (i, choice) in wiz.duplicate_choices.iter().enumerate() {
let is_sel = i == wiz.cursor;
@@ -261,30 +260,26 @@ impl<'a> Widget for ImportWizardWidget<'a> {
(false, Some(_)) => Style::default().fg(Color::DarkGray),
(false, None) => Style::default(),
};
y = draw_wrapped(buf, x, y, w, &label, style);
y = panel.wrapped(x, y, w, &label, style);
if let Some(reason) = &choice.disabled {
let dim = Style::default().fg(Color::DarkGray);
y = draw_wrapped(buf, x + 4, y, w.saturating_sub(4), reason, dim);
y = panel.wrapped(x + 4, y, w.saturating_sub(4), reason, dim);
}
}
y += 1;
buf.set_string(x, y, "Colliding records:", Style::default().fg(Color::Gray));
y += 1;
let rows: Vec<String> = wiz.collisions.iter().map(|c| c.describe()).collect();
let bottom = inner.y + inner.height - 1;
draw_scrolling(
buf,
let bottom = inner.bottom().saturating_sub(1);
panel.scrolling(
x,
y..bottom,
(y, "Colliding records"),
y.saturating_add(1)..bottom,
w,
&rows,
wiz.scroll,
" ",
Style::default(),
);
set_bottom_line(
buf,
x,
panel.bottom_line(x,
bottom,
w,
"\u{2191}\u{2193} choose Enter: select PgUp/PgDn: scroll \u{2190} back Esc: cancel",
@@ -292,7 +287,7 @@ impl<'a> Widget for ImportWizardWidget<'a> {
);
}
WizardStepKind::NameDuplicateCategory => {
buf.set_string(
panel.put(
x,
y,
"Name the category that tells duplicate records apart:",
@@ -300,10 +295,9 @@ impl<'a> Widget for ImportWizardWidget<'a> {
);
y += 1;
let input = format!("> {}\u{2588}", self.wizard.duplicate_category);
buf.set_string(x, y, truncate(&input, w), Style::default().fg(Color::Green));
panel.put(x, y, truncate(&input, w), Style::default().fg(Color::Green));
y += 2;
draw_wrapped(
buf,
panel.wrapped(
x,
y,
w,
@@ -312,17 +306,16 @@ impl<'a> Widget for ImportWizardWidget<'a> {
grid, so totals look as they would summed.",
Style::default().fg(Color::DarkGray),
);
set_bottom_line(
buf,
panel.bottom_line(
x,
inner.y + inner.height - 1,
inner.bottom().saturating_sub(1),
w,
"Enter: next \u{2190} back Esc: cancel",
Style::default().fg(Color::DarkGray),
);
}
WizardStepKind::DefineFormulas => {
buf.set_string(
panel.put(
x,
y,
"Define formulas (optional):",
@@ -332,7 +325,7 @@ impl<'a> Widget for ImportWizardWidget<'a> {
// Show existing formulas
if self.wizard.formulas.is_empty() && !self.wizard.formula_editing {
buf.set_string(
panel.put(
x,
y,
" (no formulas yet)",
@@ -341,7 +334,7 @@ impl<'a> Widget for ImportWizardWidget<'a> {
y += 1;
}
for (i, formula) in self.wizard.formulas.iter().enumerate() {
if y >= inner.y + inner.height - 5 {
if y >= inner.bottom().saturating_sub(5) {
break;
}
let is_sel = i == self.wizard.cursor && !self.wizard.formula_editing;
@@ -353,14 +346,14 @@ impl<'a> Widget for ImportWizardWidget<'a> {
} else {
Style::default().fg(Color::Green)
};
buf.set_string(x, y, truncate(&format!(" {}", formula), w), style);
panel.put(x, y, truncate(&format!(" {}", formula), w), style);
y += 1;
}
// Formula input area
if self.wizard.formula_editing {
y += 1;
buf.set_string(
panel.put(
x,
y,
"Formula (e.g., Profit = Revenue - Cost):",
@@ -368,7 +361,7 @@ impl<'a> Widget for ImportWizardWidget<'a> {
);
y += 1;
let input = format!("> {}\u{2588}", self.wizard.formula_buffer);
buf.set_string(x, y, truncate(&input, w), Style::default().fg(Color::Green));
panel.put(x, y, truncate(&input, w), Style::default().fg(Color::Green));
y += 1;
}
@@ -376,13 +369,13 @@ impl<'a> Widget for ImportWizardWidget<'a> {
let samples = &self.wizard.sample_formulas;
if !samples.is_empty() {
y += 1;
buf.set_string(x, y, "Examples:", Style::default().fg(Color::DarkGray));
panel.put(x, y, "Examples:", Style::default().fg(Color::DarkGray));
y += 1;
for sample in samples {
if y >= inner.y + inner.height - 1 {
if y >= inner.bottom().saturating_sub(1) {
break;
}
buf.set_string(
panel.put(
x,
y,
truncate(&format!(" {}", sample), w),
@@ -392,33 +385,26 @@ impl<'a> Widget for ImportWizardWidget<'a> {
}
}
let hint_y = inner.y + inner.height - 1;
let hint_y = inner.bottom().saturating_sub(1);
let hint = if self.wizard.formula_editing {
"Enter: add Esc: cancel"
} else {
"n: new formula d: delete Enter: next \u{2190} back Esc: cancel"
};
set_bottom_line(
buf,
x,
hint_y,
w,
hint,
Style::default().fg(Color::DarkGray),
);
panel.bottom_line(x, hint_y, w, hint, Style::default().fg(Color::DarkGray));
}
WizardStepKind::NameModel => {
buf.set_string(x, y, "Model name:", Style::default().fg(Color::Yellow));
panel.put(x, y, "Model name:", Style::default().fg(Color::Yellow));
y += 1;
let name_str = format!("> {}\u{2588}", self.wizard.model_name);
buf.set_string(
panel.put(
x,
y,
truncate(&name_str, w),
Style::default().fg(Color::Green),
);
y += 2;
buf.set_string(
panel.put(
x,
y,
"Enter: build \u{2190} back Esc: cancel",
@@ -430,32 +416,32 @@ impl<'a> Widget for ImportWizardWidget<'a> {
// user is about to accept, warnings included (improvise-907).
match &self.wizard.built_summary {
Some(summary) => {
buf.set_string(
panel.put(
x,
y,
truncate(summary, w),
Style::default().fg(Color::Green),
);
y += 2;
let bottom = inner.y + inner.height - 1;
draw_scrolling(
buf,
x,
y..bottom,
w,
&self.wizard.built_warnings,
self.wizard.scroll,
"! ",
Style::default().fg(Color::Yellow),
);
}
None => {
buf.set_string(x, y, "Nothing to confirm.", Style::default().fg(Color::Red))
let bottom = inner.bottom().saturating_sub(1);
let warnings = &self.wizard.built_warnings;
if !warnings.is_empty() {
panel.scrolling(
x,
(y, "Warnings"),
y.saturating_add(1)..bottom,
w,
warnings,
self.wizard.scroll,
"! ",
Style::default().fg(Color::Yellow),
);
}
}
None => panel.put(x, y, "Nothing to confirm.", Style::default().fg(Color::Red)),
}
let hint_y = inner.y + inner.height - 1;
set_bottom_line(
buf,
let hint_y = inner.bottom().saturating_sub(1);
panel.bottom_line(
x,
hint_y,
w,
@@ -469,64 +455,91 @@ impl<'a> Widget for ImportWizardWidget<'a> {
// upward from there rather than being cut (improvise-cgqt).
if let Some(msg) = &self.wizard.message {
let lines = wrap(msg, w);
let bottom = inner.y + inner.height - 1;
let bottom = inner.bottom().saturating_sub(1);
let first = bottom.saturating_sub(lines.len() as u16 - 1).max(inner.y);
for (row, line) in (first..=bottom).zip(&lines) {
set_bottom_line(buf, x, row, w, line, Style::default().fg(Color::Red));
panel.bottom_line(x, row, w, line, Style::default().fg(Color::Red));
}
}
}
}
/// Draw `text` wrapped to `w` cells from row `y`; returns the row after it.
fn draw_wrapped(buf: &mut Buffer, x: u16, y: u16, w: usize, text: &str, style: Style) -> u16 {
let lines = wrap(text, w);
for (i, line) in lines.iter().enumerate() {
buf.set_string(x, y + i as u16, line, style);
}
y + lines.len() as u16
/// The wizard's drawing surface. Every write is clipped to the panel's rows
/// and width, so no step can reach past the buffer (review B2 on #304).
struct Panel<'b> {
buf: &'b mut Buffer,
clip: Rect,
}
/// Draw `items` from `scroll` on, each wrapped whole, within `rows`. A marker
/// row says what lies above or below, so nothing is hidden without a word.
#[allow(clippy::too_many_arguments)]
fn draw_scrolling(
buf: &mut Buffer,
x: u16,
rows: std::ops::Range<u16>,
w: usize,
items: &[String],
scroll: usize,
prefix: &str,
style: Style,
) {
let dim = Style::default().fg(Color::DarkGray);
let mut y = rows.start;
if scroll > 0 && y < rows.end {
buf.set_string(x, y, format!("\u{25b2} {scroll} more above (PgUp)"), dim);
y += 1;
impl Panel<'_> {
fn put(&mut self, x: u16, y: u16, text: impl AsRef<str>, style: Style) {
let inside = (self.clip.y..self.clip.bottom()).contains(&y) && x < self.clip.right();
if inside {
let room = usize::from(self.clip.right() - x);
self.buf.set_stringn(x, y, text, room, style);
}
}
for (i, item) in items.iter().enumerate().skip(scroll) {
let lines = wrap(&format!("{prefix}{item}"), w);
let remaining = items.len() - i;
// Keep a row for the "more below" marker unless this is the last item.
let room = rows.end.saturating_sub(y) as usize;
let needed = lines.len() + usize::from(remaining > 1);
if needed > room && i > scroll {
buf.set_string(x, y, format!("\u{25bc} {remaining} more below (PgDn)"), dim);
return;
fn bottom_line(&mut self, x: u16, y: u16, w: usize, text: &str, style: Style) {
if (self.clip.y..self.clip.bottom()).contains(&y) {
set_bottom_line(self.buf, x, y, w, text, style);
}
for line in lines.iter().take(room) {
buf.set_string(x, y, line, style);
y += 1;
}
/// Draw `text` wrapped to `w` cells from row `y`; returns the row after it.
fn wrapped(&mut self, x: u16, y: u16, w: usize, text: &str, style: Style) -> u16 {
let lines = wrap(text, w);
for (i, line) in lines.iter().enumerate() {
self.put(x, y.saturating_add(i as u16), line, style);
}
y.saturating_add(lines.len() as u16)
}
/// Draw `items` from `scroll` on, each wrapped whole, in `rows`. Which of
/// them are shown is said on the `header` row, where no item can cover it.
#[allow(clippy::too_many_arguments)]
fn scrolling(
&mut self,
x: u16,
header: (u16, &str),
rows: std::ops::Range<u16>,
w: usize,
items: &[String],
scroll: usize,
prefix: &str,
style: Style,
) {
let mut y = rows.start;
let mut shown = scroll;
for item in items.iter().skip(scroll) {
let lines = wrap(&format!("{prefix}{item}"), w);
let room = usize::from(rows.end.saturating_sub(y));
if lines.len() > room && shown > scroll {
break;
}
for line in lines.iter().take(room) {
self.put(x, y, line, style);
y += 1;
}
shown += 1;
}
let (row, title) = header;
let label = match (scroll, shown) {
(0, n) if n == items.len() => format!("{title}:"),
(first, last) => format!(
"{title} {}\u{2013}{last} of {} (PgUp/PgDn):",
first + 1,
items.len()
),
};
self.put(x, row, label, Style::default().fg(Color::Gray));
}
}
/// The ONE writer for the wizard's bottom row, padding in terminal cells
/// (improvise-q0vb). Both halves are argued in `context/code/tui.md`.
fn set_bottom_line(buf: &mut Buffer, x: u16, y: u16, w: usize, text: &str, style: Style) {
let text = truncate(text, w);
let text = crate::ui::text::truncate(text, w);
let pad = " ".repeat(w.saturating_sub(UnicodeWidthStr::width(text.as_str())));
buf.set_string(x, y, format!("{text}{pad}"), style);
}
@@ -610,11 +623,19 @@ mod tests {
buf.content()
.chunks(w)
.map(|row| {
row.iter()
.map(|c| c.symbol())
.collect::<String>()
.trim_end()
.to_string()
// A double-width character's second cell is padding, not text.
let mut skip = 0;
let mut text = String::new();
for cell in row {
match skip {
0 => {
text.push_str(cell.symbol());
skip = UnicodeWidthStr::width(cell.symbol()).saturating_sub(1);
}
_ => skip -= 1,
}
}
text.trim_end().to_string()
})
.collect::<Vec<_>>()
.join("\n")
@@ -859,12 +880,39 @@ mod tests {
!first.contains("Region=Region-13"),
"precondition: it overflows\n{first}"
);
assert!(first.contains("more below"), "{first}");
assert!(first.contains("Colliding records 1\u{2013}"), "{first}");
assert!(first.contains(" of 14 (PgUp/PgDn):"), "{first}");
w.scroll(100);
let last = render(&w);
assert!(last.contains("Region=Region-13"), "{last}");
assert!(last.contains("more above"), "{last}");
assert!(
last.contains("Colliding records 14\u{2013}14 of 14"),
"{last}"
);
}
/// Codex P2 on PR #304: an item that fills the list exactly left the "more
/// below" marker on the hint row, drawn over. `context/test-evidence/tui.md`.
#[test]
fn a_first_item_that_fills_the_list_still_says_more_follow() {
let area = Rect::new(0, 0, 10, 6);
let mut buf = Buffer::empty(area);
let mut panel = Panel {
buf: &mut buf,
clip: area,
};
let items = ["aaaa bbbb cccc".to_string(), "dddd".to_string()];
panel.scrolling(0, (0, "L"), 1..4, 4, &items, 0, "", Style::default());
set_bottom_line(&mut buf, 0, 4, 10, "hint", Style::default());
let drawn = buf_text(&buf);
let rows: Vec<&str> = drawn.lines().collect();
assert_eq!(
rows[1..4],
["aaaa", "bbbb", "cccc"],
"the first item whole:\n{drawn}"
);
assert!(rows[0].starts_with("L 1\u{2013}1 of 2"), "{drawn}");
}
#[test]
@@ -892,6 +940,175 @@ mod tests {
assert_drawn_whole(&drawn, &reason);
}
fn render_at(w: &ImportWizard, width: u16, height: u16) -> String {
let area = Rect::new(0, 0, width, height);
let mut buf = Buffer::empty(area);
let cache = project::wizard_cache(w);
ImportWizardWidget::new(&cache).render(area, &mut buf);
buf_text(&buf)
}
/// The reported import: twelve categories and a collision.
fn wizard_at_the_cap() -> ImportWizard {
let recs: Vec<serde_json::Value> = (0..3)
.chain([0])
.map(|r| {
let mut m = serde_json::Map::new();
for i in 0..12 {
m.insert(format!("c{i:02}"), json!(format!("v{r}")));
}
m.insert("amount".into(), json!(1.0));
serde_json::Value::Object(m)
})
.collect();
let mut w = ImportWizard::new(serde_json::Value::Array(recs));
w.advance();
assert!(matches!(w.step, WizardStep::ResolveDuplicates), "setup");
w
}
/// Review B1 on PR #304: the cap reason carries a `←`, three bytes wide,
/// and a byte-measured bottom row cut it. `context/test-evidence/tui.md`.
#[test]
fn a_multibyte_message_is_drawn_whole_at_every_width() {
let mut w = wizard_at_the_cap();
w.move_cursor(1);
w.choose_duplicate();
let reason = w
.message
.clone()
.expect("setup: choosing Separate at the cap");
assert!(reason.contains('\u{2190}'), "setup: {reason}");
let messages = [
reason,
"Région 日本語 🎉 naïve café — ← → Skip, Measure, Sum".repeat(3),
];
for msg in messages {
w.message = Some(msg.clone());
for width in 30..=100u16 {
let drawn = render_at(&w, width, 30);
let flat: String = drawn.lines().collect();
assert!(
!flat.contains('\u{2026}'),
"width {width} truncated:\n{drawn}"
);
assert_drawn_whole(&drawn, &msg);
}
}
}
/// Every step the wizard can reach, as the TUI would draw it: each a fresh
/// wizard walked by `advance`, `choose` or `build` in turn.
fn every_step() -> Vec<ImportWizard> {
#[derive(Clone, Copy)]
enum Op {
Advance,
Separate,
Sum,
Build,
}
let colliding = || {
ImportWizard::new(json!([
{"Region": "East", "Revenue": 1.0}, {"Region": "East", "Revenue": 2.0}
]))
};
let walk = |mut w: ImportWizard, ops: &[Op]| {
for op in ops {
match op {
Op::Advance => w.advance(),
Op::Separate => {
w.move_cursor(1);
w.choose_duplicate();
}
Op::Sum => w.choose_duplicate(),
Op::Build => w.build(),
}
}
w
};
use Op::*;
vec![
ImportWizard::new(json!([])),
ImportWizard::new(json!({
"sales": [{"Region": "East", "Revenue": 1}],
"costs": [{"Region": "West", "Cost": 2}],
})),
wizard(),
walk(wizard(), &[Advance]),
walk(wizard(), &[Advance, Advance]),
walk(wizard(), &[Advance, Advance, Advance]),
walk(wizard(), &[Advance, Advance, Advance, Build]),
walk(colliding(), &[Advance]),
walk(colliding(), &[Advance, Separate]),
walk(colliding(), &[Advance, Sum, Advance, Build]),
wizard_at_the_cap(),
walk(wizard_at_the_cap(), &[Separate]),
{
let mut w = walk(wizard(), &[Advance, Advance]);
for formula in ["Profit = Revenue - Cost", "Margin = Profit / Revenue"] {
w.start_formula_edit();
formula.chars().for_each(|c| w.push_formula_char(c));
w.confirm_formula();
}
w.start_formula_edit();
w.push_formula_char('M');
w
},
]
}
/// Review B2 on PR #304: the duplicates step wrote past the panel and
/// ratatui panicked. Every step, every small size. `context/test-evidence/tui.md`.
#[test]
fn no_step_panics_on_a_small_terminal() {
let steps = every_step();
let kinds: Vec<String> = steps.iter().map(|w| format!("{:?}", w.step)).collect();
for kind in [
"Preview",
"SelectArrayPath",
"ReviewProposals",
"ConfigureDates",
"ResolveDuplicates",
"NameDuplicateCategory",
"DefineFormulas",
"NameModel",
"Done",
] {
assert!(
kinds.iter().any(|k| k == kind),
"setup: no {kind} in {kinds:?}"
);
}
let sweep = |cache: &crate::protocol::WizardCache, kind: &str| {
let mut panics = vec![];
for width in 20..=100u16 {
for height in 3..=20u16 {
let drawn = std::panic::catch_unwind(|| {
let area = Rect::new(0, 0, width, height);
ImportWizardWidget::new(cache).render(area, &mut Buffer::empty(area));
});
if drawn.is_err() {
panics.push(format!("{kind} {width}x{height}"));
}
}
}
panics
};
let caches: Vec<_> = steps.iter().map(project::wizard_cache).collect();
let panics: Vec<String> = std::thread::scope(|scope| {
let running: Vec<_> = caches
.iter()
.zip(&kinds)
.map(|(cache, kind)| scope.spawn(move || sweep(cache, kind)))
.collect();
running
.into_iter()
.flat_map(|t| t.join().expect("the sweep itself does not panic"))
.collect()
});
assert!(panics.is_empty(), "{} panics: {:?}", panics.len(), panics);
}
const PREVIEW_STEP: &str = concat!(
"┌ Import Wizard — Preview ─────────────────────────────────────────────────────┐\n",
"│Array of 0 records. Sample keys: │\n",
src/ui/text.rs
+11
View File
@@ -104,6 +104,17 @@ mod tests {
}
}
/// Review B1 on PR #304: bytes are never fewer than cells, so a byte
/// measure only wraps early. These fit in cells and not in bytes.
#[test]
fn wrapping_measures_cells_not_bytes() {
let s = "\u{2190}\u{2190}\u{2190} \u{e9}\u{e9}";
assert_eq!(s.len(), 14, "setup: fourteen bytes");
assert_eq!(wrap(s, 6), [s], "six cells fit on one line of six");
let cjk = "\u{65e5}\u{672c} ab";
assert_eq!(wrap(cjk, 7), [cjk], "seven cells, ten bytes");
}
/// An indented option or list row keeps its indent, on every line.
#[test]
fn wrapping_keeps_a_leading_indent_on_every_line() {