From 2aab457c44fdbf225ac1618c73f3e465abe65360 Mon Sep 17 00:00:00 2001 From: "claude@clouddev1" Date: Thu, 28 May 2026 12:16:28 +0000 Subject: [PATCH] =?UTF-8?q?feat:=20DSL=E2=86=92SQL=20teaching=20echo=20?= =?UTF-8?q?=E2=80=94=20=C2=A74=20styled-runs=20polish=20(ADR-0038)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Lands the last open item on ADR-0038: the de-emphasised styled-runs rendering treatment for the echo + every category-3 prose line. The echoed SQL now reads as code — the dimmed `Executing SQL:` label plus the SQL portion lexed and coloured the same way the input echo treats user-typed input (ADR-0028 §5 styled-runs over input_render::lex_to_runs in advanced mode). Category-3 prose lines (the DontConvert caveat and the existing illuminating `client_side.*` notes — shortid auto-fill, type-conversion transforms) all render dimmed too, per §6's "de-emphasised prose line" wording, so every cat-3 line is visually consistent. * New `OutputKind::TeachingEcho` variant + a custom branch in `ui::render_output_line` mirroring the OutputKind::Echo input-echo path: strip the canonical `Executing SQL:` prefix, render it with `theme.muted`, then lex the rest in `Mode::Advanced` and emit one span per token. Tag stays `[system]` for visual consistency with other system output. * New `OutputStyleClass::Hint` styled-runs class, resolved to `theme.muted` in `output_span_style`. Used for the cat-3 prose lines (dont_convert caveat + the existing client_side notes). * New const `crate::echo::TEACHING_ECHO_LABEL = "Executing SQL: "` — the byte boundary the ui.rs branch needs is fixed (an i18n template can't provide that), so the label moves from i18n to a constant. The `echo.executing_sql` i18n key is retired (en-US.yaml + keys.rs); a comment in en-US.yaml points future locales at re-introducing it if needed. * App-side helpers: `push_teaching_echo(sql)` builds the TeachingEcho line; `push_category_three_prose(text)` builds a System line with a whole-text Hint span. `note_ok_summary` and `handle_dsl_change_column_success` / `handle_dsl_add_column_success` use these instead of plain `note_system` for the echo, the caveat, and the illuminating notes. Existing tests pass unchanged — text content is the same; only styling changes. New tests pin the polish: * `ui::tests::teaching_echo_line_renders_dim_prefix_and_lexed_sql` asserts the TeachingEcho rendering produces a dim prefix span + keyword-coloured SQL spans (confirming the lexer ran in advanced mode). * `ui::tests::category_three_prose_line_renders_all_dim` pins the whole-text Hint coverage. * `ui::tests::hint_class_resolves_to_muted_foreground` pins the theme resolution across both light and dark. * `app::tests::polished_echo_carries_teaching_echo_kind_and_caveat_a_hint_span` pins the App-side wiring (kind + styled_runs shape). Tests: 2019 passed / 0 failed / 1 ignored (pre-existing); clippy clean (`--all-targets -D warnings`, nursery). ADR-0038 is now feature-complete — every catalogue row implemented, round-tripped, AND polished per §4. --- src/app.rs | 156 ++++++++++++++++++++++++++++---- src/echo.rs | 11 +++ src/friendly/keys.rs | 2 - src/friendly/strings/en-US.yaml | 12 ++- src/ui.rs | 132 ++++++++++++++++++++++++++- 5 files changed, 286 insertions(+), 27 deletions(-) diff --git a/src/app.rs b/src/app.rs index baf9477..4ff1832 100644 --- a/src/app.rs +++ b/src/app.rs @@ -28,6 +28,13 @@ pub enum OutputKind { Echo, System, Error, + /// The DSL → SQL teaching echo (ADR-0038 §4). Visually a `[system]` + /// line, but rendered with a custom path: a dimmed `Executing SQL:` + /// prefix followed by the SQL re-lexed through `input_render:: + /// lex_to_runs_in_mode(Advanced)` — same syntax highlighting the + /// input echo gets, so the suggested SQL reads like code (ADR-0028 + /// §5 styled-runs). + TeachingEcho, } /// The semantic style class of an [`OutputSpan`] (ADR-0028 §5). @@ -49,6 +56,11 @@ pub enum OutputStyleClass { /// index because none existed; the strongest "add an index /// here" signal. AutomaticIndex, + /// De-emphasised text — `Executing SQL:` prefix on teaching + /// echo lines (ADR-0038 §4), the DontConvert caveat, and + /// every `[client-side]` category-3 prose note (ADR-0038 §6). + /// Resolves to `theme.muted`. + Hint, } /// A styled span of an output line: a byte range over the @@ -1420,16 +1432,16 @@ impl App { verb = command.verb(), subject = command.display_subject() )); - // ADR-0038: the DSL → SQL teaching echo, beneath `[ok]`. Set on - // the success event when a DSL-form command ran in an advanced - // effective mode (ADR-0037); `None` otherwise. De-emphasised - // (styled-runs polish per ADR-0038 §4 still pending). One line - // per statement — single-statement echoes render one line; - // multi-statement (`drop column --cascade`, `add relationship - // --create-fk`) render one per entry (ADR-0038 §6 category 2). + // ADR-0038 §4: the DSL → SQL teaching echo, beneath `[ok]`. + // Set on the success event when a DSL-form command ran in an + // advanced effective mode (ADR-0037); `None` otherwise. One + // `OutputKind::TeachingEcho` line per statement (§6 category + // 2): the dimmed `Executing SQL:` prefix + the SQL portion + // re-lexed in advanced mode for syntax highlighting — see + // `ui::render_output_line`'s `TeachingEcho` branch. if let Some(lines) = self.pending_echo.take() { for line in lines { - self.note_system(crate::t!("echo.executing_sql", sql = line)); + self.push_teaching_echo(&line); } } } @@ -1493,12 +1505,12 @@ impl App { result: AddColumnResult, ) { self.note_ok_summary(command); - // ADR-0018 §9: emit auto-fill note(s) before the - // structure render, so the pedagogical "the tool did - // this for you" line is in the user's eye-line next to - // the success summary. + // ADR-0018 §9 / ADR-0038 §6 category 3: emit auto-fill note(s) + // before the structure render so the pedagogical "the tool did + // this for you" line is in the user's eye-line next to the + // success summary. De-emphasised per §6 (illuminating prose). for note in result.client_side_notes { - self.note_system(note); + self.push_category_three_prose(note); } for line in crate::output_render::render_structure(&result.description) { self.note_system(line); @@ -1541,6 +1553,9 @@ impl App { // When both transformations and auto-fills happen // in the same operation, both note lines are // emitted in order (ADR-0018 §9 explicit rule). + // ADR-0038 §6 category 3: both lines are illuminating prose + // (the SQL line is equivalent; the note merely reveals a + // value-add SQL doesn't show). De-emphasised. if note.transformed > 0 { let line = if note.lossy > 0 { crate::t!( @@ -1554,7 +1569,7 @@ impl App { count = note.transformed ) }; - self.note_system(line); + self.push_category_three_prose(line); } if note.auto_filled > 0 { let kind = match note.auto_fill_kind { @@ -1562,7 +1577,7 @@ impl App { Some(crate::db::AutoFillKind::ShortId) => "shortid", None => "auto-generated", }; - self.note_system(crate::t!( + self.push_category_three_prose(crate::t!( "client_side.auto_fill_transition", count = note.auto_filled, kind = kind @@ -1574,9 +1589,10 @@ impl App { // nearest SQL but *not* equivalent (the only Bucket A caveat — // every other category-3 line is illuminating). Sits between // the client-side notes and the structure render so it reads - // alongside the echo, not after the table view. + // alongside the echo, not after the table view. De-emphasised + // prose per §6. if dont_convert_caveat { - self.note_system(crate::t!("client_side.dont_convert_caveat")); + self.push_category_three_prose(crate::t!("client_side.dont_convert_caveat")); } for line in crate::output_render::render_structure(&result.description) { self.note_system(line); @@ -2224,6 +2240,46 @@ impl App { self.push_multiline(text.into(), OutputKind::System); } + /// Push one teaching-echo line (ADR-0038 §4 styled-runs polish). + /// The text is `""`; the `TeachingEcho` + /// kind triggers `ui::render_output_line`'s custom branch, which + /// renders the prefix dimmed (`theme.muted`) and lexes the SQL in + /// advanced mode for syntax highlighting — the same treatment the + /// input echo receives. + fn push_teaching_echo(&mut self, sql: &str) { + let text = format!("{}{sql}", crate::echo::TEACHING_ECHO_LABEL); + self.push_output(OutputLine { + text, + kind: OutputKind::TeachingEcho, + mode_at_submission: self.mode, + styled_runs: None, + }); + } + + /// Push one category-3 prose line (ADR-0038 §6) — the + /// `--dont-convert` caveat and the existing illuminating + /// `client_side.*` notes (shortid auto-fill, type-conversion + /// transforms). The whole line text is rendered dimmed + /// (`OutputStyleClass::Hint` → `theme.muted`); the `[system]` + /// tag keeps its kind styling. De-emphasised, per §6. + fn push_category_three_prose(&mut self, text: impl Into) { + let text = text.into(); + let runs = if text.is_empty() { + Vec::new() + } else { + vec![OutputSpan { + byte_range: (0, text.len()), + class: OutputStyleClass::Hint, + }] + }; + self.push_output(OutputLine::styled( + text, + OutputKind::System, + self.mode, + runs, + )); + } + fn note_error(&mut self, text: impl Into) { self.push_multiline(text.into(), OutputKind::Error); } @@ -3066,6 +3122,72 @@ mod tests { assert_echo_beneath_ok(&app, "ALTER TABLE T ALTER COLUMN c SET DATA TYPE text"); } + #[test] + fn polished_echo_carries_teaching_echo_kind_and_caveat_a_hint_span() { + // ADR-0038 §4 styled-runs polish: the App-side wiring places + // every echo line as an OutputKind::TeachingEcho (so + // `ui::render_output_line`'s custom branch fires — dim prefix + // + lexed SQL) and every category-3 prose line as a System + // line with a single Hint span covering the whole text (so + // the body renders dimmed via `output_span_style`). + use crate::db::ChangeColumnTypeResult; + let mut app = App::new(); + app.update(AppEvent::DslChangeColumnSucceeded { + command: Command::ChangeColumnType { + table: "T".to_string(), + column: "c".to_string(), + ty: Type::Int, + mode: crate::dsl::ChangeColumnMode::DontConvert, + }, + result: ChangeColumnTypeResult { + description: sample_description("T"), + client_side: None, + }, + echo: Some(vec![ + "ALTER TABLE T ALTER COLUMN c SET DATA TYPE int".to_string(), + ]), + dont_convert_caveat: true, + }); + + // The echo line is a TeachingEcho. + let echo_line = app + .output + .iter() + .find(|l| l.text.contains("Executing SQL:")) + .expect("an echo line"); + assert_eq!( + echo_line.kind, + OutputKind::TeachingEcho, + "echo line carries TeachingEcho so ui.rs fires the dim-prefix + lex-rest branch", + ); + // The echo line carries no styled_runs payload — the custom + // ui.rs branch builds its own spans from the kind alone. + assert!( + echo_line.styled_runs.is_none(), + "echo line uses kind-driven custom rendering, not styled-runs", + ); + + // The caveat is a System line with a single Hint span covering + // the whole text — the whole prose body renders dim. + let caveat_line = app + .output + .iter() + .find(|l| l.text.contains("`--dont-convert` kept the stored values")) + .expect("a caveat line"); + assert_eq!(caveat_line.kind, OutputKind::System); + let runs = caveat_line + .styled_runs + .as_ref() + .expect("caveat carries a styled-runs payload"); + assert_eq!(runs.len(), 1); + assert_eq!(runs[0].class, OutputStyleClass::Hint); + assert_eq!( + runs[0].byte_range, + (0, caveat_line.text.len()), + "the dim span covers the entire prose body", + ); + } + #[test] fn change_column_dont_convert_renders_the_caveat_between_notes_and_structure() { // ADR-0038 §6 category 3 (Phase 3): when `change column … diff --git a/src/echo.rs b/src/echo.rs index 646444b..8bd6adb 100644 --- a/src/echo.rs +++ b/src/echo.rs @@ -21,6 +21,17 @@ use crate::dsl::command::{ }; use crate::dsl::value::Value; +/// The dimmed `Executing SQL:` prefix on a teaching-echo line +/// (ADR-0038 §4 styled-runs polish). +/// +/// The App appends the SQL to this prefix when building a +/// `OutputKind::TeachingEcho` line; `ui::render_output_line` strips +/// it back off, renders the prefix dimmed (`theme.muted`), and lexes +/// the rest in advanced mode for syntax highlighting — same treatment +/// the input echo gets. The trailing space is part of the constant so +/// the SQL position is predictable for the lexer's byte ranges. +pub const TEACHING_ECHO_LABEL: &str = "Executing SQL: "; + /// The teaching echo for a command submitted under `mode`, or `None`. /// /// Fires only in an advanced effective mode (`AdvancedPersistent` / diff --git a/src/friendly/keys.rs b/src/friendly/keys.rs index 0f8ac4c..de22dab 100644 --- a/src/friendly/keys.rs +++ b/src/friendly/keys.rs @@ -497,8 +497,6 @@ pub const KEYS_AND_PLACEHOLDERS: &[(&str, &[&str])] = &[ ("ok.rows_inserted", &["count"]), ("ok.rows_updated", &["count"]), ("ok.summary", &["verb", "subject"]), - // ---- DSL → SQL teaching echo (ADR-0038) ---- - ("echo.executing_sql", &["sql"]), // ---- Client-side success notes (ADR-0017 §6, ADR-0018 §9) ---- ("client_side.auto_fill_add_serial", &["count"]), ("client_side.auto_fill_add_shortid", &["count"]), diff --git a/src/friendly/strings/en-US.yaml b/src/friendly/strings/en-US.yaml index 1111fb8..c5cdd72 100644 --- a/src/friendly/strings/en-US.yaml +++ b/src/friendly/strings/en-US.yaml @@ -874,11 +874,13 @@ db: Cannot add this CHECK to `{table}.{column}`: {total} row(s) do not satisfy `{rule}`. # ---- DSL command success summaries (ADR-0019 §9 sweep) -------------- -# DSL → SQL teaching echo (ADR-0038): the equivalent advanced-mode SQL, -# rendered beneath `[ok]` for a DSL-form command run in an advanced -# effective mode (ADR-0037). -echo: - executing_sql: "Executing SQL: {sql}" +# (The DSL → SQL teaching echo's `Executing SQL:` prefix used to live +# here as `echo.executing_sql`; with the ADR-0038 §4 styled-runs polish +# the line is now built from a hardcoded constant in `crate::echo` +# because the dim-prefix + lex-the-rest rendering path in +# `ui::render_output_line` needs a fixed byte boundary the i18n +# template couldn't provide. Re-introduce a key here if a non-English +# locale lands.) ok: # Generic `[ok] ` header used for every diff --git a/src/ui.rs b/src/ui.rs index 0aac888..aa429e6 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -606,7 +606,7 @@ fn approximate_wrapped_rows_from_output( Mode::Simple => "[simple] ".len(), Mode::Advanced => "[advanced] ".len(), }, - OutputKind::System => "[system] ".len(), + OutputKind::System | OutputKind::TeachingEcho => "[system] ".len(), OutputKind::Error => "[error] ".len(), }; let total = tag_len + line.text.chars().count(); @@ -625,6 +625,11 @@ const fn output_span_style(class: OutputStyleClass, theme: &Theme) -> Style { OutputStyleClass::AutomaticIndex => Style::new() .fg(theme.warning) .add_modifier(Modifier::BOLD), + // ADR-0038 §4 / §6: de-emphasised text — the `Executing SQL:` + // prefix and every category-3 prose line (caveat + the + // existing `client_side.*` notes). `theme.muted` is the + // established dim foreground. + OutputStyleClass::Hint => Style::new().fg(theme.muted), } } @@ -635,7 +640,7 @@ fn render_output_line<'a>(line: &'a OutputLine, theme: &Theme) -> Line<'a> { }; let tag = match line.kind { OutputKind::Echo => format!("[{}] ", line.mode_at_submission.label().to_lowercase()), - OutputKind::System => "[system] ".to_string(), + OutputKind::System | OutputKind::TeachingEcho => "[system] ".to_string(), OutputKind::Error => "[error] ".to_string(), }; @@ -665,6 +670,34 @@ fn render_output_line<'a>(line: &'a OutputLine, theme: &Theme) -> Line<'a> { // Echo body without the expected prefix, or any non-echo // line, falls through to the plain rendering below. + // ADR-0038 §4 styled-runs polish — the DSL → SQL teaching echo + // gets the same syntactic treatment as the input echo, but with + // a dimmed `Executing SQL: ` prefix instead of the mode label, + // and the SQL portion lexed in `Mode::Advanced` (the echoed SQL + // is always advanced syntax, regardless of submission mode). + if line.kind == OutputKind::TeachingEcho + && let Some(rest) = line.text.strip_prefix(crate::echo::TEACHING_ECHO_LABEL) + { + let prefix_len = crate::echo::TEACHING_ECHO_LABEL.len(); + let mut spans: Vec> = Vec::with_capacity(2 + rest.len() / 4); + spans.push(Span::styled(tag, tag_style)); + spans.push(Span::styled( + &line.text[..prefix_len], + Style::default().fg(theme.muted), + )); + for run in + crate::input_render::lex_to_runs_in_mode(rest, theme, Mode::Advanced) + { + spans.push(Span::styled( + &rest[run.byte_range.0..run.byte_range.1], + run.style, + )); + } + return Line::from(spans); + } + // A TeachingEcho line missing the canonical prefix is a builder + // bug; it falls through to the plain rendering below. + // ADR-0028 §5: a line carrying a styled-runs payload is // rendered span-by-span, each run's semantic class resolved // to a colour from the active theme. The tag keeps its @@ -685,7 +718,11 @@ fn render_output_line<'a>(line: &'a OutputLine, theme: &Theme) -> Line<'a> { let body_style = match line.kind { OutputKind::Echo => Style::default().fg(theme.fg), - OutputKind::System => Style::default().fg(theme.system), + // TeachingEcho without a prefix falls back to `system` styling + // — the kind itself only signals "render with the dim+lex + // custom path above"; reaching here means the builder produced + // a malformed line, so degrade gracefully rather than crash. + OutputKind::System | OutputKind::TeachingEcho => Style::default().fg(theme.system), OutputKind::Error => Style::default().fg(theme.error), }; Line::from(vec![ @@ -1094,6 +1131,95 @@ mod tests { assert_eq!(rendered.spans[2].style.fg, Some(theme.fg)); } + #[test] + fn hint_class_resolves_to_muted_foreground() { + // ADR-0038 §4 / §6: the dim style class used for the + // `Executing SQL:` prefix, the DontConvert caveat, and every + // category-3 prose note. Pins the `theme.muted` resolution + // across both palettes. + for theme in [Theme::dark(), Theme::light()] { + let style = output_span_style(OutputStyleClass::Hint, &theme); + assert_eq!( + style.fg, + Some(theme.muted), + "Hint must resolve to theme.muted on {:?} background", + theme.background, + ); + } + } + + #[test] + fn teaching_echo_line_renders_dim_prefix_and_lexed_sql() { + // ADR-0038 §4 styled-runs polish: a TeachingEcho line is laid + // out as [tag][dim prefix][lexed SQL spans]. The tag stays the + // mode tint; the `Executing SQL: ` prefix is `theme.muted`; + // the SQL portion is re-lexed in advanced mode so it picks up + // keyword / identifier / literal colours. + let theme = Theme::dark(); + let line = OutputLine { + text: format!( + "{}{}", + crate::echo::TEACHING_ECHO_LABEL, + "CREATE TABLE T (id serial PRIMARY KEY)" + ), + kind: OutputKind::TeachingEcho, + mode_at_submission: Mode::Advanced, + styled_runs: None, + }; + let rendered = render_output_line(&line, &theme); + // [system] tag, then the dim prefix, then ≥1 SQL spans. + assert!(rendered.spans.len() >= 3, "tag + prefix + sql: {:?}", rendered.spans); + assert_eq!(rendered.spans[0].content.as_ref(), "[system] "); + assert_eq!(rendered.spans[1].content.as_ref(), crate::echo::TEACHING_ECHO_LABEL); + assert_eq!( + rendered.spans[1].style.fg, + Some(theme.muted), + "prefix is dimmed (theme.muted)", + ); + // At least one SQL span carries a keyword colour — `CREATE` is + // the leading keyword and gets `tok_keyword`. Pinning this + // also confirms the lexer ran in advanced mode (the bare + // `CREATE` keyword is only highlighted past the entry word in + // advanced — ADR-0030 §8). + let has_keyword_span = rendered + .spans + .iter() + .any(|s| s.style.fg == Some(theme.tok_keyword)); + assert!( + has_keyword_span, + "expected at least one keyword-coloured SQL span: {:?}", + rendered.spans + ); + } + + #[test] + fn category_three_prose_line_renders_all_dim() { + // ADR-0038 §6: the existing illuminating client_side notes and + // the new --dont-convert caveat are de-emphasised prose. A + // styled-runs payload with a single Hint span over the whole + // text yields one dim body span (plus the [system] tag). + let theme = Theme::dark(); + let text = "[client-side] 5 row(s) were transformed".to_string(); + let line = OutputLine::styled( + text.clone(), + OutputKind::System, + Mode::Advanced, + vec![OutputSpan { + byte_range: (0, text.len()), + class: OutputStyleClass::Hint, + }], + ); + let rendered = render_output_line(&line, &theme); + assert_eq!(rendered.spans.len(), 2, "tag + one Hint span"); + assert_eq!(rendered.spans[0].content.as_ref(), "[system] "); + assert_eq!(rendered.spans[1].content.as_ref(), text.as_str()); + assert_eq!( + rendered.spans[1].style.fg, + Some(theme.muted), + "the whole prose line is dim", + ); + } + #[test] fn candidate_line_colours_mixed_mode_continuations() { // ADR-0035 §4i (e): when a shared entry word's completions mix