From c8e19ea47f11305d4831ad2e081841e5e5ed9aec Mon Sep 17 00:00:00 2001 From: vasilito Date: Sun, 21 Jun 2026 00:50:05 +0300 Subject: [PATCH] =?UTF-8?q?PLAN=20=C2=A717:=20dialog=20consistency=20asses?= =?UTF-8?q?sment=20(44/46=20unified,=204-item=20P1-P4=20backlog)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- local/recipes/tui/tlc/PLAN.md | 179 +++++++++++++++++++++++++++++++++- 1 file changed, 177 insertions(+), 2 deletions(-) diff --git a/local/recipes/tui/tlc/PLAN.md b/local/recipes/tui/tlc/PLAN.md index 9a295128c7..0486169973 100644 --- a/local/recipes/tui/tlc/PLAN.md +++ b/local/recipes/tui/tlc/PLAN.md @@ -2,8 +2,8 @@ **Status:** Architecture chosen. Implementation in progress. Phases 0–8 substantially complete. Phases 14a, 14b, 15a, 15b (partial), 15c (partial), 15d, 14e, 16–29 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>` +- `pub fn render_button_row(spans: &mut Vec>, specs: &[ButtonSpec<'_>], default_idx: Option)` +- `pub fn button_width(spec: ButtonSpec<'_>) -> u16` +- `pub struct ButtonSpec<'a> { pub kind: ButtonKind, pub label: &'a str, pub hotkey: Option }` + +**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 (LOW–MEDIUM)** — + 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`; `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.