diff --git a/src/app.rs b/src/app.rs index dae22a3..2863382 100644 --- a/src/app.rs +++ b/src/app.rs @@ -646,6 +646,44 @@ impl App { } } + /// The input view the **live-feedback** walkers (completion, ambient + /// hint, validity verdict, highlight overlays) should see, plus the + /// byte offset stripped from the front and the cursor mapped into the + /// view. + /// + /// Under the `:` one-shot escape (ADR-0003) the buffer carries a + /// leading `:` (and an auto-inserted space) that is *not* advanced + /// SQL — submission already strips it before parsing, but the live + /// feedback did not, so the walker bailed at the `:` and resolved + /// nothing (no completion / hint, a spurious error overlay). This + /// returns the stripped SQL exactly as submission sees it, so the + /// feedback matches a real advanced-mode session. `offset` maps any + /// walker-returned byte position (completion `replaced_range`, + /// overlay spans) back to real-buffer coordinates. + /// + /// For every non-one-shot input this is the identity + /// `(&input, cursor, 0)`. + #[must_use] + pub fn feedback_view(&self) -> (&str, usize, usize) { + if matches!(self.effective_mode(), EffectiveMode::AdvancedOneShot) { + // The first non-whitespace char is the `:` (per + // `effective_mode`); strip up to and including it, then any + // following whitespace — mirroring submission's + // `trimmed[1..].trim()`. + let leading_ws = self.input.len() - self.input.trim_start().len(); + let mut offset = leading_ws + 1; // past the `:` + while offset < self.input.len() + && self.input.as_bytes()[offset].is_ascii_whitespace() + { + offset += 1; + } + let view = &self.input[offset..]; + let cursor = self.input_cursor.saturating_sub(offset).min(view.len()); + return (view, cursor, offset); + } + (&self.input, self.input_cursor, 0) + } + /// The validity-indicator verdict for the current input /// (ADR-0027 §3). `None` when the input would run clean. /// @@ -667,11 +705,10 @@ impl App { EffectiveMode::AdvancedPersistent | EffectiveMode::AdvancedOneShot => Mode::Advanced, }; - crate::dsl::walker::input_verdict_in_mode( - &self.input, - Some(&self.schema_cache), - mode, - ) + // Strip the `:` one-shot prefix so the walker verdicts the SQL + // itself, not the escape marker (which it can't parse). + let (view, _cursor, _offset) = self.feedback_view(); + crate::dsl::walker::input_verdict_in_mode(view, Some(&self.schema_cache), mode) } /// Process one event from the runtime, mutating state and @@ -1399,13 +1436,7 @@ impl App { } fn start_or_complete_at(&mut self, multi_start_idx: usize) { - let cursor = self.input_cursor.min(self.input.len()); - let Some(comp) = crate::completion::candidates_at_cursor_in_mode( - &self.input, - cursor, - &self.schema_cache, - self.effective_mode().as_mode(), - ) else { + let Some(comp) = self.completion_for_feedback() else { return; }; if comp.candidates.len() == 1 { @@ -1417,13 +1448,7 @@ impl App { } fn start_or_complete_last(&mut self) { - let cursor = self.input_cursor.min(self.input.len()); - let Some(comp) = crate::completion::candidates_at_cursor_in_mode( - &self.input, - cursor, - &self.schema_cache, - self.effective_mode().as_mode(), - ) else { + let Some(comp) = self.completion_for_feedback() else { return; }; if comp.candidates.len() == 1 { @@ -1434,6 +1459,22 @@ impl App { } } + /// Completion at the cursor, computed against the `:`-stripped + /// feedback view (ADR-0003 one-shot) with its `replaced_range` + /// mapped back to real-buffer coordinates so `commit_*` edit the + /// right span. Identity for non-one-shot input (offset 0). + fn completion_for_feedback(&self) -> Option { + let (view, view_cursor, offset) = self.feedback_view(); + let mut comp = crate::completion::candidates_at_cursor_in_mode( + view, + view_cursor.min(view.len()), + &self.schema_cache, + self.effective_mode().as_mode(), + )?; + comp.replaced_range = (comp.replaced_range.0 + offset, comp.replaced_range.1 + offset); + Some(comp) + } + /// Single-candidate commit: insert " " (with trailing /// space) and DO NOT create a memo. The user can keep /// typing or press Tab again to fresh-complete at the new @@ -4976,6 +5017,86 @@ mod tests { assert_eq!(app.effective_mode(), EffectiveMode::AdvancedPersistent); } + /// Build a two-table cache (`Orders(id, customer_id)` + + /// `Customers(id, name)`) for the `:` one-shot SQL-feedback tests. + fn install_join_schema(app: &mut App) { + use crate::completion::TableColumn; + use crate::dsl::types::Type; + app.schema_cache.tables = vec!["Orders".into(), "Customers".into()]; + app.schema_cache.table_columns.insert( + "Orders".into(), + vec![TableColumn::new("id", Type::Serial), TableColumn::new("customer_id", Type::Int)], + ); + app.schema_cache.table_columns.insert( + "Customers".into(), + vec![TableColumn::new("id", Type::Serial), TableColumn::new("name", Type::Text)], + ); + for t in app.schema_cache.tables.clone() { + for c in &app.schema_cache.table_columns[&t] { + app.schema_cache.columns.push(c.name.clone()); + } + } + } + + #[test] + fn colon_one_shot_gives_sql_completion_the_stripped_view() { + // Bug (manual testing): the `:` one-shot escape (ADR-0003) left + // the leading `:` in the buffer passed to the live SQL feedback, + // so the walker bailed at `:` and Tab completed nothing — while + // the identical line in full `mode advanced` completed. Now the + // feedback view strips the `:`, so both behave the same. + let body = "select c.name from Orders o join Customers c on c.id=o.cu"; + + // Full advanced mode: completes `o.cu` → `o.customer_id`. + let mut adv = App::new(); + adv.mode = Mode::Advanced; + install_join_schema(&mut adv); + type_str(&mut adv, body); + adv.update(key(KeyCode::Tab)); + assert!( + adv.input.ends_with("o.customer_id "), + "full advanced should complete: {:?}", + adv.input + ); + + // `:` one-shot from simple mode: must complete the same way, and + // the `:` prefix must be preserved in the buffer. + let mut one = App::new(); + one.mode = Mode::Simple; + install_join_schema(&mut one); + one.update(key(KeyCode::Char(':'))); + type_str(&mut one, body); + assert_eq!(one.effective_mode(), EffectiveMode::AdvancedOneShot); + one.update(key(KeyCode::Tab)); + assert!( + one.input.trim_start().starts_with(':'), + "the `:` prefix is kept: {:?}", + one.input + ); + assert!( + one.input.ends_with("o.customer_id "), + "`:` one-shot must complete the SQL column too: {:?}", + one.input + ); + } + + #[test] + fn colon_one_shot_validity_is_clean_for_a_valid_query() { + // A *valid* `:`-prefixed query must not light the `[ERR]` + // indicator (the walker used to choke on the `:` and always + // report Error). + let mut app = App::new(); + install_join_schema(&mut app); + app.update(key(KeyCode::Char(':'))); + type_str(&mut app, "select name from Customers"); + assert_eq!( + app.input_validity_verdict(), + None, + "a valid one-shot query should verdict clean, got {:?}", + app.input_validity_verdict(), + ); + } + #[test] fn effective_mode_flips_to_one_shot_when_colon_typed_in_simple_mode() { let mut app = App::new(); diff --git a/src/input_render.rs b/src/input_render.rs index 7358ff6..47efe00 100644 --- a/src/input_render.rs +++ b/src/input_render.rs @@ -84,16 +84,60 @@ pub fn render_input_runs_in_mode( cache: &crate::completion::SchemaCache, mode: Mode, ) -> Vec { - let mut runs = lex_to_runs_in_mode(input, theme, mode); + // Identity feedback view — highlight/overlay the whole input. + render_input_runs_feedback(input, cursor_byte, theme, cache, mode, input, cursor_byte, 0) +} + +/// [`render_input_runs_in_mode`] with a separate **feedback view** for +/// the walker-driven highlighting and overlays. +/// +/// Under the `:` one-shot escape (ADR-0003) the buffer carries a leading +/// `:` that is not advanced SQL; `view` is the stripped SQL (and +/// `view_cursor` the cursor within it) so the walker highlights and +/// diagnoses the SQL itself, while the `:` prefix renders as plain text. +/// `offset` is the byte length stripped from the front — base runs and +/// overlay positions are shifted by it back into `input` coordinates. +/// Callers without a one-shot escape pass `(input, cursor, 0)` (what +/// [`render_input_runs_in_mode`] does). +#[must_use] +#[allow(clippy::too_many_arguments)] +pub fn render_input_runs_feedback( + input: &str, + cursor_byte: usize, + theme: &Theme, + cache: &crate::completion::SchemaCache, + mode: Mode, + view: &str, + view_cursor: usize, + offset: usize, +) -> Vec { + // Base highlighting runs over the SQL view, shifted into buffer + // coordinates; the stripped prefix (the `:` + space) renders as + // plain foreground text. + let mut runs: Vec = if offset == 0 { + lex_to_runs_in_mode(input, theme, mode) + } else { + let mut r = vec![StyledRun { + byte_range: (0, offset), + style: ratatui::style::Style::default().fg(theme.fg), + }]; + r.extend(lex_to_runs_in_mode(view, theme, mode).into_iter().map(|run| { + StyledRun { + byte_range: (run.byte_range.0 + offset, run.byte_range.1 + offset), + ..run + } + })); + r + }; if let InputState::DefiniteErrorAt(pos) = - classify_parse_result(parse_command_with_schema_in_mode(input, cache, mode)) + classify_parse_result(parse_command_with_schema_in_mode(view, cache, mode)) { - overlay_error(&mut runs, pos, theme); + overlay_error(&mut runs, pos + offset, theme); } if let Some(inv) = - crate::completion::invalid_ident_at_cursor_in_mode(input, cursor_byte, cache, mode) + crate::completion::invalid_ident_at_cursor_in_mode(view, view_cursor, cache, mode) { - overlay_error(&mut runs, inv.range.0, theme); + overlay_error(&mut runs, inv.range.0 + offset, theme); } // Schema-aware diagnostics (ADR-0027 §2): unknown table / // column (ERROR), or a dubious comparison (WARNING), is @@ -101,12 +145,12 @@ pub fn render_input_runs_in_mode( // so a problem the user has typed past stays visible. The // mode-aware walk picks up the SQL-specific diagnostics from // ADR-0032 in advanced mode. - for diag in walker::input_diagnostics_in_mode(input, Some(cache), mode) { + for diag in walker::input_diagnostics_in_mode(view, Some(cache), mode) { let colour = match diag.severity { walker::Severity::Error => theme.tok_error, walker::Severity::Warning => theme.warning, }; - overlay_span(&mut runs, diag.span, colour); + overlay_span(&mut runs, (diag.span.0 + offset, diag.span.1 + offset), colour); } inject_cursor(&mut runs, input, cursor_byte, theme); runs @@ -1108,6 +1152,50 @@ mod tests { assert!(reversed(&runs[0])); } + #[test] + fn one_shot_colon_highlights_the_sql_and_overlays_no_error() { + // ADR-0003 `:` one-shot: the SQL after the `:` must highlight and + // diagnose like real advanced mode — the `:` prefix renders as + // plain text and a valid query carries no error overlay (the old + // path let the walker choke on the `:` and mark it red). + use crate::completion::{SchemaCache, TableColumn}; + use crate::dsl::types::Type; + let theme = dark(); + let mut cache = SchemaCache::default(); + cache.tables.push("Customers".into()); + cache.columns.push("name".into()); + cache + .table_columns + .insert("Customers".into(), vec![TableColumn::new("name", Type::Text)]); + let input = ": select name from Customers"; + let view = "select name from Customers"; + let offset = 2; // ": " + let runs = render_input_runs_feedback( + input, + input.len(), + &theme, + &cache, + Mode::Advanced, + view, + view.len(), + offset, + ); + assert!( + runs.iter().all(|r| r.style.fg != Some(theme.tok_error)), + "a valid one-shot query must carry no error overlay: {runs:?}", + ); + assert!( + runs.iter() + .any(|r| r.byte_range.0 == offset && r.style.fg == Some(theme.tok_keyword)), + "the `select` keyword (past the `: ` prefix) is keyword-coloured: {runs:?}", + ); + assert_eq!( + runs.first().unwrap().byte_range.0, + 0, + "the `:` prefix is rendered from byte 0", + ); + } + #[test] fn keyword_token_takes_keyword_colour() { let theme = dark(); diff --git a/src/ui.rs b/src/ui.rs index e4df268..409cf9e 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -1438,12 +1438,19 @@ fn render_input_one_row( let offset = input_scroll_offset(line_cols, cursor_col, tw, app.input_scroll_offset); app.input_scroll_offset = offset; - let runs = crate::input_render::render_input_runs_in_mode( + // Strip the `:` one-shot prefix for the SQL highlighting/overlays + // (ADR-0003); the `:` itself renders as plain text. Identity for + // non-one-shot input. + let (fb_view, fb_cursor, fb_off) = app.feedback_view(); + let runs = crate::input_render::render_input_runs_feedback( &app.input, cursor, theme, &app.schema_cache, mode_for_render, + fb_view, + fb_cursor, + fb_off, ); let spans = runs_to_spans(&app.input, &runs); @@ -1507,12 +1514,19 @@ fn render_input_two_rows( let offset = input_scroll_offset(line_cols, cursor_col, capacity, app.input_scroll_offset); app.input_scroll_offset = offset; - let runs = crate::input_render::render_input_runs_in_mode( + // Strip the `:` one-shot prefix for the SQL highlighting/overlays + // (ADR-0003); the `:` itself renders as plain text. Identity for + // non-one-shot input. + let (fb_view, fb_cursor, fb_off) = app.feedback_view(); + let runs = crate::input_render::render_input_runs_feedback( &app.input, cursor, theme, &app.schema_cache, mode_for_render, + fb_view, + fb_cursor, + fb_off, ); let cells = expand_runs_to_cells(&app.input, &runs); let len = cells.len(); @@ -1621,23 +1635,6 @@ fn runs_to_spans<'a>( .collect() } -/// Strip a leading one-shot `:` sigil (and the whitespace after -/// it) from `input`, returning the advanced command slice and the -/// cursor remapped into it. Mirrors `App::submit`'s `:` handling -/// so the hint panel hints at the command, not the sigil -/// (ADR-0022 Amendment 1). Used only when the effective mode is -/// `AdvancedOneShot`, where `input` is guaranteed to start (after -/// any leading whitespace) with `:`. -fn strip_one_shot_prefix(input: &str, cursor: usize) -> (&str, usize) { - let lead_ws = input.len() - input.trim_start().len(); - let after_colon = lead_ws + 1; // skip the `:` - let ws_after = input[after_colon..].len() - input[after_colon..].trim_start().len(); - let prefix_len = (after_colon + ws_after).min(input.len()); - let effective = &input[prefix_len..]; - let effective_cursor = cursor.saturating_sub(prefix_len).min(effective.len()); - (effective, effective_cursor) -} - /// Resolve the Hint panel body into its rendered lines, pre-wrapped /// to the panel's inner width and clamped to `max_rows` with an /// ellipsis backstop (issue #12). `max_rows` is the geometry-fixed row @@ -1679,14 +1676,9 @@ fn resolve_hint_lines( // In one-shot advanced mode (`:` prefix in simple mode) the // raw input carries the `:` sigil, which is not part of the - // grammar. Strip it for the ambient computation so the hint - // reflects the advanced command — mirroring `App::submit`. - let (hint_input, hint_cursor) = match app.effective_mode() { - EffectiveMode::AdvancedOneShot => { - strip_one_shot_prefix(&app.input, app.input_cursor) - } - _ => (app.input.as_str(), app.input_cursor), - }; + // grammar. The shared feedback view strips it so the hint reflects + // the advanced command — mirroring `App::submit` (ADR-0003). + let (hint_input, hint_cursor, _off) = app.feedback_view(); let ambient = crate::input_render::ambient_hint_in_mode( hint_input, hint_cursor,