PLAN §17: dialog consistency assessment (44/46 unified, 4-item P1-P4 backlog)

Comprehensive audit of every dialog in tlc — render path, button
source, shadow source, hotkey consistency. Verifies the Phase 16-18
work brought 44/46 dialog surfaces onto the unified render_popup
path (33 filemanager + 5 editor/render + 1 ops/progress), with MC
rounded borders + drop shadow + skin-driven palette inherited
automatically.

Identifies the remaining 7% (the four high-priority items in §17.4):

  P1 (HIGH) widget/dialog.rs:194 — Dialog::render uses raw Clear +
       own Block instead of render_popup, so Dialog::info / confirm /
       confirm_all render flat-border popups with NO drop shadow.
       Only shadow regression in the unified stack. ~5-10 LOC.

  P2 (HIGH) filemanager/overwrite_dialog.rs:138-181 — hand-rolled
       Y/N/All/Skip/Abort colored Span legend. Single most visually
       inconsistent button strip in the codebase. Migrate to
       render_button_row of 5 ButtonSpec. ~25-40 LOC.

  P3 (MED)  filemanager/confirm_dialog.rs:186-191 — '[ Save ] [ Cancel ]'
       Paragraph with theme.hidden. Migrate to 2-button row. ~8-15 LOC.

  P4 (MED)  editor/menubar.rs:520 — F9 dropdown still uses hand-rolled
       Block + Clear instead of render_popup. Filemanager menubar
       dropdown was migrated (c032c9a787); editor was not. ~5-10 LOC.

Plus lower-priority items:
  P5 editor SaveBeforeClose Y/N/Esc legend migration
  P6 viewer/editor SaveBeforeClose unification
  P7 (optional) ~15 decorative key-legend hints migrated to
     ButtonKind::Narrow rows

§17.3 documents the intentional non-targets (full-screen TreeDialog,
F9 menu-bar top rows, viewer render_prompt_overlay) so future
auditors know not to migrate them.

§17.5 codifies the three hotkey patterns (Action OK/Cancel,
Yes/No confirm, Yes/No/All/Skip confirm) and the locale keys
that drive them.

§17.6 sets the acceptance criteria: after P1-P4, grep for raw
Clear should return 5 sites, all defensible. Zero shadow
regressions. Zero hand-rolled button strips that look like
real buttons.

Tests: 1180 passing, 0 failing (no code changes, audit only).
This commit is contained in:
2026-06-21 00:50:05 +03:00
parent 6f99194049
commit c8e19ea47f
+177 -2
View File
@@ -2,8 +2,8 @@
**Status:** Architecture chosen. Implementation in progress. Phases 08 substantially complete.
Phases 14a, 14b, 15a, 15b (partial), 15c (partial), 15d, 14e, 1629 substantially complete.
**Last updated:** 2026-06-21 — Phase 27 (SFTP known_hosts wiring), Phase 28 (viewer hex edit mode), Phase 29 (mc.ext Include resolver), PLAN.md sync.
**Date:** 2026-06-12 (initial) · 2026-06-13 (rename + comprehensive review + audit fixes) · 2026-06-19 (bug fixes, standalone binaries, syntax highlighter, parity audit reconciliation) · 2026-06-20 (Phase 16, Phase 17, Phase 18, Phase 19, Phase 20, Phase 21, Phase 22, Phase 23, Phase 24, Phase 25, Phase 26) · 2026-06-21 (Phase 27, Phase 28, Phase 29)
**Last updated:** 2026-06-21 — Phase 27/28/29 sync + §17 dialog consistency assessment (44/46 dialogs unified, 4-item P1-P4 refactor backlog).
**Date:** 2026-06-12 (initial) · 2026-06-13 (rename + comprehensive review + audit fixes) · 2026-06-19 (bug fixes, standalone binaries, syntax highlighter, parity audit reconciliation) · 2026-06-20 (Phase 16, Phase 17, Phase 18, Phase 19, Phase 20, Phase 21, Phase 22, Phase 23, Phase 24, Phase 25, Phase 26) · 2026-06-21 (Phase 27, Phase 28, Phase 29, §17 dialog audit)
**Branch:** `0.2.4`
**Decision authority:** User selected Option A (Pure Rust TLC) on 2026-06-12.
**Scope:** Reimplement ALL of Midnight Commander (MC 4.8.33) in pure Rust.
@@ -1412,6 +1412,167 @@ All 1108 existing tests pass unchanged.
---
## 17. DIALOG CONSISTENCY ASSESSMENT (2026-06-21, post-Phase 18 audit)
Comprehensive audit of every dialog in tlc: which render path each uses, whether buttons
are unified, and whether shadows are inherited. Goal: confirm the Phase 16-18 migration
is complete and identify the remaining bespoke surfaces.
### 17.1 Inventory at a glance
- **44 dialogs use `terminal::popup::render_popup`** (the unified path): all popup chrome
comes from a single source — rounded borders, MC-matching drop shadow via
`render_drop_shadow`, title bar from `[dialog] dtitle`.
- 33 in `src/filemanager/*.rs` (copy, move, mkdir, link, delete, info, permission, owner,
find, compare, jobs, hotlist, usermenu, skin_dialog, screen_list, edit_history,
external_panelize, filtered_view, quickcd_dialog, pattern_dialog, filter_dialog,
connection_dialog, connection_manager, vfs_list, quit_dialog, panel_options,
layout_dialog, sort_dialog, encoding_dialog, help, config_dialog, chattr_dialog, exec)
- 5 in `src/editor/render.rs` (completion, save-before-close, standard prompt, history, help)
- 1 in `src/ops/progress.rs`
- Plus `widget/dialog.rs::Dialog::render` is a 45th site that builds a similar shell but
does NOT use `render_popup` — see §17.4 P1.
- **5 surfaces use bespoke `frame.render_widget(Clear, ...)`** by design:
- `terminal/popup.rs` (the renderer itself)
- `widget/dialog.rs:194` (the `Dialog` widget — see §17.4 P1, this is the one to fix)
- `editor/menubar.rs:426` top bar (full-width by design — see §17.3 non-targets)
- `filemanager/menubar.rs:260` top bar (full-width by design)
- `filemanager/tree.rs:304` (full-screen tree browser — see §17.3 non-targets)
- **2 surfaces are full-screen / no popup chrome** (intentional, do NOT migrate):
- `filemanager/tree.rs` TreeDialog — owns the entire terminal
- the F9 menu-bar top rows in editor + filemanager
### 17.2 Button rendering consistency
The unified button API lives at `src/widget/button.rs`:
- `pub enum ButtonKind { Normal, Default, Narrow }`
- `pub fn render_button(spec: &ButtonSpec<'_>, focused: bool) -> Vec<Span<'static>>`
- `pub fn render_button_row(spans: &mut Vec<Span<'static>>, specs: &[ButtonSpec<'_>], default_idx: Option<usize>)`
- `pub fn button_width(spec: ButtonSpec<'_>) -> u16`
- `pub struct ButtonSpec<'a> { pub kind: ButtonKind, pub label: &'a str, pub hotkey: Option<char> }`
**5 dialogs use `render_button_row` correctly:**
- `copy_dialog` (OK/Cancel, O/C, focused=0)
- `move_dialog` (OK/Cancel, O/C, focused=0)
- `mkdir_dialog` (OK/Cancel, O/C)
- `link` (action/Cancel, action[0]/C)
- `delete_dialog` (Yes/No, Y/N, focused=0)
**2 dialogs have hand-rolled button strips that look like real buttons and should be migrated:**
| File | Lines | What it does | Refactor |
|---|---|---|---|
| `filemanager/overwrite_dialog.rs` | 138-181 | Y/N/All/Skip/Abort legend built from `Span::styled` with hand-picked `theme.executable`/`theme.error`/`theme.title_bg`/`theme.warning` colors. Most elaborate non-migrated strip in the codebase. | Replace with 5-element `render_button_row` of `ButtonSpec`s (Yes Default, No Normal, YesAll Normal, NoAll Normal, Abort Normal). |
| `filemanager/confirm_dialog.rs` | 186-191 | `" [ Save ] [ Cancel ] "` rendered as a single `Paragraph` with `theme.hidden` color. Visually mimics buttons but uses no brackets and no skin palette. | Replace with 2-element `render_button_row` (Save Default, Cancel Normal). |
**~15 dialogs have decorative key-legend hints (not real buttons)** hand-built from
`Span::styled`: `external_panelize`, `jobs`, `screen_list`, `filtered_view`,
`connection_manager`, `skin_dialog`, `find`, `permission`, `owner`, `exec`, `config_dialog`,
`editor/render.rs:469-519` SaveBeforeClose Y/N/Esc, `viewer/mod.rs:672` SaveBeforeQuit.
These are key hints (Tab/Enter/Esc), not clickable buttons. Optional: migrate to
`ButtonKind::Narrow` rows for visual parity. Pure cosmetic; no functional impact.
### 17.3 Non-targets (intentionally bespoke)
- **`filemanager/tree.rs:304`** — full-screen tree browser. The whole `area` is a tree.
No popup shell, no shadow, no buttons. Migrating would be wrong.
- **`editor/menubar.rs:426` and `filemanager/menubar.rs:260`** — the F9 top menu bar.
Full-width by design (the bar IS the navigation surface). The dropdown menus
underneath both use `render_popup` (filemanager was migrated in commit `c032c9a787`;
editor was migrated as part of Phase 18 but the dropdown itself uses raw `Block`+`Clear`
instead of `render_popup` — see §17.4 P4).
- **`viewer/mod.rs:664` `render_prompt_overlay`** — the viewer Search/SearchBackward/
GotoLine/SaveBeforeQuit prompt is a deliberate 1-row inline header bar (matches MC's
viewer prompt convention: the bar replaces the viewer's header row, no popup chrome).
Migrating to `render_popup` would break the viewer layout.
- **`viewer/hex_edit.rs:64-74`** mini-prompt for hex-edit overflow text. Inline by design.
### 17.4 Refactor backlog (prioritized)
1. **P1 — `widget/dialog.rs:194` (HIGH)**`Dialog::render` builds its own popup shell
with `frame.render_widget(Clear, popup)` + its own `Block::default().borders(Borders::ALL)`
+ its own `Paragraph`. It does NOT call `terminal::popup::render_popup` and therefore
does NOT call `render_drop_shadow`. **Result**: `Dialog::info`/`Dialog::confirm`/
`Dialog::confirm_all` are flat-border popups with NO drop shadow — the only shadow
regression in the unified stack. No production callers use these constructors yet
(the 16 dialogs migrated in Phase 16-18 went directly to `render_popup`), but this is
latent drift waiting to happen if anyone adds a new caller.
**Refactor**: make `Dialog::render` call `terminal::popup::render_popup` for the shell,
keep `render_button_row` for the button bar. Estimated 5-10 LOC.
2. **P2 — `filemanager/overwrite_dialog.rs:138-181` (HIGH)** — replace the hand-rolled
Y/N/A/S/Esc colored-span legend with a 5-element `render_button_row` of
`ButtonSpec`s. **Single most visually inconsistent button strip in the codebase.**
Estimated 25-40 LOC + 1-2 tests asserting the rendered output contains MC bracket shapes.
3. **P3 — `filemanager/confirm_dialog.rs:186-191` (MEDIUM)** — replace
`" [ Save ] [ Cancel ] "` Paragraph with a 2-element `render_button_row`
(Save Default, Cancel Normal). Estimated 8-15 LOC + 1 test.
4. **P4 — `editor/menubar.rs:520` dropdown (MEDIUM)** — the editor F9 dropdown still
uses its own `Block` + own `Clear` instead of `render_popup`. The filemanager menubar
dropdown (committed as `c032c9a787`) was migrated. **Direct inconsistency**: the
two F9 menus diverge in chrome. Estimated 5-10 LOC, no test needed (the dropdown
behavior is unchanged).
5. **P5 — `editor/render.rs:469-519` SaveBeforeClose Y/N/Esc legend (LOWMEDIUM)**
replace hand-rolled Spans with `render_button_row` for consistency with the migrated
file-manager dialogs. Estimated 20-30 LOC + 1 test. Defensible to leave as-is because
the prompt label and Y/N/Esc are tightly coupled (the prompt text mentions
"Save before close?" then "Y/N/Esc" — they're a single sentence, not a button bar).
6. **P6 — viewer/editor SaveBeforeClose unification (LOW)** — viewer uses inline
header-bar text; editor uses popup. **These are two different render strategies for
the same conceptual prompt.** Options: (a) leave as-is, document as intentional
divergence (viewer's row-0 prompt vs editor's centered popup); (b) migrate viewer
SaveBeforeQuit to use the editor's popup path; (c) introduce a `ConfirmationPrompt`
trait that both impls. Option (a) is the lowest-risk choice and is recommended unless
user feedback indicates otherwise.
7. **P7 — decorative key-legend strips (~15 files, OPTIONAL)** — convert Tab/Enter/Esc
legends to rows of `ButtonKind::Narrow` `render_button_row` for full visual parity
with the action-button dialogs. Pure cosmetic; no functional impact. **Recommend
deferring** until user-visible polish is requested.
### 17.5 Hotkey policy (codified)
All migrated dialogs follow one of three MC-matching patterns:
| Pattern | Hotkeys | Dialogs |
|---|---|---|
| **Action (OK/Cancel)** | Default = action-label[0] (typically `O` for OK, `C` for create), Cancel = `C` | `copy_dialog`, `move_dialog`, `mkdir_dialog`, `link` |
| **Yes/No confirm** | Default = `Y`, Cancel = `N` | `delete_dialog` |
| **Yes/No/All/Skip confirm** | Default = `Y`, No = `N`, YesAll = `A`, NoAll = `S`, Abort = `Esc` | `overwrite_dialog` (currently hand-rolled; will be unified in P2) |
Locale-driven via `dialog_action_ok`/`cancel`/`yes`/`no` keys (all 6 supported locales
have parity, enforced by `i18n_all_catalogues_have_same_keys`).
### 17.6 Acceptance criteria
After P1-P4 land, a `grep "frame.render_widget(Clear" src/` should return exactly:
- `terminal/popup.rs` (the renderer itself)
- `widget/dialog.rs:194` (gone after P1)
- `editor/menubar.rs:426` top bar (intentional full-width)
- `filemanager/menubar.rs:260` top bar (intentional full-width)
- `filemanager/tree.rs:304` (full-screen, intentional)
Five raw `Clear` sites total, all defensible. Zero shadow regressions. Zero hand-rolled
button strips that look like real buttons.
After P5 lands: the SaveBeforeClose prompt matches the action-dialog button style
visually. After P7 (optional): full visual parity across all 44 unified dialogs.
### 17.7 Migration completion statement
The Phase 16-18 work brought **44/46 dialog surfaces onto the unified `render_popup` path**
with MC-matching drop shadow, MC bracket-shaped buttons, and skin-driven palette.
The two remaining bespoke surfaces (`filemanager/tree.rs`, the two menu-bar top rows)
are intentional non-chrome. The §17.4 backlog is the final 7% of dialog unification work;
the four priority items (P1-P4) are each small (5-40 LOC) and can be done in a single
sitting.
---
## Changelog
- **2026-06-20** — **Phase 19 column (rectangular) block operations**: TLC editor gains MC's column-block mode. New `SelectionMode::{Stream, Column}` enum on `Cursor`; column anchor stored in parallel `column_anchor: Option<usize>`; `column_selection_rect()` returns normalized `ColumnRect {start_line, start_col, end_line, end_col}` (start_line ≤ end_line, start_col ≤ end_col); `selected_text()` for column mode joins rows with `\n`; `delete_selection()` strips rectangle columns from each line, leaves rest intact; `start_selection()`/`start_column_selection()` are mutually exclusive (switching clears the other anchor). Alt+Arrow / Alt+PgUp / Alt+PgDn start/extend column selection (MC `MarkColumn*` bindings). F5/F6/F8 work transparently in column mode via unified `selected_text()` / `delete_selection()`. Renderer uses new `line_selection_range()` helper to compute per-line byte ranges from either stream or column source; column cells get `marked_bg` highlight. PLAN.md §15d row 21 marked ✅ Done. 1119 tests pass.
@@ -1498,3 +1659,17 @@ All 1108 existing tests pass unchanged.
- **2026-06-21** — **Test count**: 1180 tests, 0 failures.
- **2026-06-21** — **Binary sizes**: `tlc` 5.4 MB, `tlcedit` 3.9 MB, `tlcview` 3.8 MB.
- **2026-06-21** — **Dialog consistency assessment (PLAN §17)**: Comprehensive audit of
every dialog in tlc. Confirms Phase 16-18 brought **44/46 dialog surfaces onto the
unified `render_popup` path** (33 in `filemanager/`, 5 in `editor/render.rs`, 1 in
`ops/progress.rs`; the 45th `widget/dialog.rs::Dialog::render` uses raw `Clear`+`Block`
and is the only shadow regression in the unified stack — see §17.4 P1). The two
remaining bespoke surfaces are intentional (`filemanager/tree.rs` full-screen, the
two menu-bar top rows). 5 dialogs use `render_button_row` correctly; 2 have hand-rolled
button strips that look like real buttons and should migrate (`overwrite_dialog` Y/N/A/S/Esc
colored legend, `confirm_dialog` Save/Cancel text). ~15 dialogs have decorative key-legend
hints (optional migration). PLAN §17.4 backlog ranked: P1 widget/dialog.rs shadow fix,
P2 overwrite_dialog Y/N/A/S/Esc migration, P3 confirm_dialog Save/Cancel migration,
P4 editor menubar dropdown migration, P5 editor SaveBeforeClose legend, P6
viewer/editor SaveBeforeClose unification, P7 optional decorative hint strips.