diff --git a/src/app.rs b/src/app.rs index 6e682f2..a3c45a3 100644 --- a/src/app.rs +++ b/src/app.rs @@ -705,7 +705,7 @@ impl App { &self.schema_cache, )?; let idx = start_idx % comp.candidates.len(); - let inserted = format!("{} ", comp.candidates[idx]); + let inserted = format!("{} ", comp.candidates[idx].text); let original_text = self.input[comp.replaced_range.0..comp.replaced_range.1].to_string(); self.input @@ -746,7 +746,7 @@ impl App { memo: crate::completion::LastCompletion, idx: usize, ) -> crate::completion::LastCompletion { - let new_inserted = format!("{} ", memo.candidates[idx]); + let new_inserted = format!("{} ", memo.candidates[idx].text); let (start, end) = memo.inserted_range; self.input.replace_range(start..end, &new_inserted); let new_end = start + new_inserted.len(); diff --git a/src/completion.rs b/src/completion.rs index 6dd1bd9..bdb1de5 100644 --- a/src/completion.rs +++ b/src/completion.rs @@ -46,6 +46,23 @@ impl SchemaCache { } } +/// A single Tab-insertable item with its source (so the +/// renderer can colour keywords differently from schema +/// identifiers, and so the ordering can group keywords first). +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct Candidate { + pub text: String, + pub kind: CandidateKind, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum CandidateKind { + /// One of the parser's expected keywords. + Keyword, + /// A schema entity (table, column, relationship). + Identifier, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct Completion { /// Byte range in `input` to be replaced when a candidate @@ -55,10 +72,9 @@ pub struct Completion { /// Partial prefix the user has typed at the cursor. Empty /// when the cursor is at a token boundary. pub partial_prefix: String, - /// Fitting candidates, alphabetised, deduplicated. Each - /// candidate is the bare keyword text (no backticks). When - /// inserted, the renderer adds a trailing space. - pub candidates: Vec, + /// Fitting candidates, ordered keywords-first then + /// identifiers, alphabetised within each group, deduplicated. + pub candidates: Vec, } /// Compute what would happen if the user pressed Tab right @@ -107,26 +123,47 @@ pub fn candidates_at_cursor( } let lowered_prefix = partial_prefix.to_lowercase(); + let matches_prefix = |s: &str| s.to_lowercase().starts_with(&lowered_prefix); - // Source 1: keyword candidates. - let keyword_iter = expected + // Source 1: keyword candidates from the parser's + // expected-set. + let mut keywords: Vec = expected .iter() .filter_map(|item| strip_backticks(item)) - .filter_map(|name| Keyword::from_word(name).map(|_| name.to_string())); - - // Source 2: schema identifiers (TableName / Column / - // RelationshipName slots). We collect across all matching - // slots. `NewName` slots return empty lists from the cache. - let schema_iter = expected.iter().filter_map(|item| { - IdentSlot::from_expected_label(item).map(|slot| cache.for_slot(slot)) - }); - - let mut candidates: Vec = keyword_iter - .chain(schema_iter.flat_map(|names| names.iter().cloned())) - .filter(|name| name.to_lowercase().starts_with(&lowered_prefix)) + .filter_map(|name| Keyword::from_word(name).map(|_| name.to_string())) + .filter(|name| matches_prefix(name)) .collect(); - candidates.sort(); - candidates.dedup(); + keywords.sort(); + keywords.dedup(); + + // Source 2: schema identifiers — accumulated across every + // matching known-set slot. `NewName` slots return `&[]`. + let mut identifiers: Vec = expected + .iter() + .filter_map(|item| IdentSlot::from_expected_label(item)) + .flat_map(|slot| cache.for_slot(slot).iter().cloned()) + .filter(|name| matches_prefix(name)) + .collect(); + identifiers.sort(); + identifiers.dedup(); + // If an identifier shares its name with a keyword candidate + // (rare in practice), the keyword wins — keywords are + // grammar; the user can name a table the same thing but + // resolving collisions in the user's favour would create + // ambiguity in the live render. + identifiers.retain(|name| !keywords.contains(name)); + + // Keywords first (grammar parts read before content), + // identifiers after. Within each group, alphabetical. + let mut candidates: Vec = Vec::with_capacity(keywords.len() + identifiers.len()); + candidates.extend(keywords.into_iter().map(|text| Candidate { + text, + kind: CandidateKind::Keyword, + })); + candidates.extend(identifiers.into_iter().map(|text| Candidate { + text, + kind: CandidateKind::Identifier, + })); if candidates.is_empty() { return None; @@ -253,8 +290,11 @@ pub struct LastCompletion { /// The text that was at `inserted_range` *before* any /// completion was applied — restored by Esc / Backspace. pub original_text: String, - /// Cycle list, fixed at memo-creation time. - pub candidates: Vec, + /// Cycle list, fixed at memo-creation time. Each candidate + /// carries its kind so the hint panel keeps the + /// keyword-vs-identifier colour coding stable across cycle + /// transitions. + pub candidates: Vec, /// Which `candidates[i]` is currently visible. pub selection_idx: usize, } @@ -280,12 +320,25 @@ mod tests { fn cands(input: &str, cursor: usize) -> Vec { candidates_at_cursor(input, cursor, &SchemaCache::default()) - .map_or_else(Vec::new, |c| c.candidates) + .map_or_else(Vec::new, |c| c.candidates.into_iter().map(|c| c.text).collect()) } fn cands_with(input: &str, cursor: usize, cache: &SchemaCache) -> Vec { candidates_at_cursor(input, cursor, cache) - .map_or_else(Vec::new, |c| c.candidates) + .map_or_else(Vec::new, |c| c.candidates.into_iter().map(|c| c.text).collect()) + } + + fn cand_kinds_with( + input: &str, + cursor: usize, + cache: &SchemaCache, + ) -> Vec<(String, CandidateKind)> { + candidates_at_cursor(input, cursor, cache).map_or_else(Vec::new, |c| { + c.candidates + .into_iter() + .map(|c| (c.text, c.kind)) + .collect() + }) } #[test] @@ -377,7 +430,9 @@ mod tests { .expect("some completion"); assert_eq!(comp.replaced_range, (0, 3)); assert_eq!(comp.partial_prefix, "cre"); - assert_eq!(comp.candidates, vec!["create".to_string()]); + assert_eq!(comp.candidates.len(), 1); + assert_eq!(comp.candidates[0].text, "create"); + assert_eq!(comp.candidates[0].kind, CandidateKind::Keyword); } #[test] @@ -388,6 +443,48 @@ mod tests { assert_eq!(comp.partial_prefix, ""); } + #[test] + fn keywords_come_before_identifiers_in_candidate_order() { + // "add column " has both keyword candidates (`to`, + // `table`) and schema-identifier candidates. Per the + // user feedback after stage 8: keywords first + // (alphabetical within), identifiers second + // (alphabetical within). + let cache = SchemaCache { + tables: vec!["Customers".to_string(), "Orders".to_string()], + ..SchemaCache::default() + }; + let kinds = cand_kinds_with("add column ", 11, &cache); + // Expect: ["table", "to"] keywords, then ["Customers", + // "Orders"] identifiers. + assert_eq!( + kinds, + vec![ + ("table".to_string(), CandidateKind::Keyword), + ("to".to_string(), CandidateKind::Keyword), + ("Customers".to_string(), CandidateKind::Identifier), + ("Orders".to_string(), CandidateKind::Identifier), + ], + ); + } + + #[test] + fn keyword_wins_when_keyword_text_collides_with_schema_name() { + // Pathological: a table named "table". Keywords + // dominate the slot — the user can still reference + // their table via different syntax. + let cache = SchemaCache { + tables: vec!["table".to_string(), "Customers".to_string()], + ..SchemaCache::default() + }; + let kinds = cand_kinds_with("add column ", 11, &cache); + // `table` appears once, as a keyword (not duplicated + // as identifier). + let table_entries: Vec<_> = kinds.iter().filter(|(t, _)| t == "table").collect(); + assert_eq!(table_entries.len(), 1); + assert_eq!(table_entries[0].1, CandidateKind::Keyword); + } + // ---- SchemaCache + identifier completion (stage 8c) ---- #[test] @@ -526,12 +623,19 @@ mod tests { assert!(cs.is_empty(), "got {cs:?}"); } + fn keyword_cand(text: &str) -> Candidate { + Candidate { + text: text.to_string(), + kind: CandidateKind::Keyword, + } + } + #[test] fn last_completion_next_idx_wraps_around() { let mut memo = LastCompletion { inserted_range: (0, 0), original_text: String::new(), - candidates: vec!["a".into(), "b".into(), "c".into()], + candidates: vec![keyword_cand("a"), keyword_cand("b"), keyword_cand("c")], selection_idx: 0, }; assert_eq!(memo.next_idx(), 1); @@ -546,7 +650,7 @@ mod tests { let mut memo = LastCompletion { inserted_range: (0, 0), original_text: String::new(), - candidates: vec!["a".into(), "b".into(), "c".into()], + candidates: vec![keyword_cand("a"), keyword_cand("b"), keyword_cand("c")], selection_idx: 0, }; assert_eq!(memo.prev_idx(), 2); diff --git a/src/input_render.rs b/src/input_render.rs index e646c2d..f2671a7 100644 --- a/src/input_render.rs +++ b/src/input_render.rs @@ -133,13 +133,15 @@ pub enum AmbientHint { /// identifier or punctuation slot), and definite-error /// states with optional usage template. Prose(String), - /// Multi-candidate (or single-candidate keyword) - /// completion at the cursor. Stage 8b renders these as - /// styled spans with the selected item highlighted (if - /// any) and `<` / `>` scroll markers when items overflow - /// the panel width. + /// Multi-candidate (or single-candidate) completion at + /// the cursor. Each item carries its kind so the + /// renderer can colour keywords differently from + /// schema-identifiers (post-stage-8 user feedback). + /// The selected item — if any — gets bold + brighter + /// colour; `<` / `>` markers appear at the edges when + /// items overflow the panel width. Candidates { - items: Vec, + items: Vec, /// Index into `items` of the currently-inserted Tab /// candidate (per the live `LastCompletion` memo), or /// `None` if the user hasn't pressed Tab yet. @@ -162,16 +164,28 @@ pub fn ambient_hint( if input.trim().is_empty() { return None; } - // First check for candidates at the cursor (keywords + - // schema identifiers). When any exist the user can Tab to - // insert one, and the panel surfaces them directly — this - // wins over the prose IncompleteAtEof framing because the - // candidate list is more actionable. + // Mid-cycle through Tab candidates: the memo carries the + // candidate list captured when Tab was first pressed, plus + // the current selection_idx. While the memo is alive the + // hint shows that exact list — recomputing at the + // post-insert cursor would whiplash the panel through "what + // comes next at the new cursor" between cycles. Closes + // the user-reported #4 in stage-8 testing. + if let Some(m) = memo { + return Some(AmbientHint::Candidates { + items: m.candidates.clone(), + selected: Some(m.selection_idx), + }); + } + // No memo: fall back to candidates_at_cursor. When any + // exist the user can Tab to insert one, and the panel + // surfaces them directly — this wins over the prose + // IncompleteAtEof framing because the candidate list is + // more actionable. if let Some(comp) = crate::completion::candidates_at_cursor(input, cursor, cache) { - let selected = memo.map(|m| m.selection_idx); return Some(AmbientHint::Candidates { items: comp.candidates, - selected, + selected: None, }); } // Invalid identifier: cursor sits in a known-set slot but @@ -471,7 +485,9 @@ mod tests { fn cands_hint(input: &str, cursor: usize) -> Option> { match ambient_hint(input, cursor, None, &empty_cache()) { - Some(AmbientHint::Candidates { items, .. }) => Some(items), + Some(AmbientHint::Candidates { items, .. }) => { + Some(items.into_iter().map(|c| c.text).collect()) + } _ => None, } } @@ -562,27 +578,61 @@ mod tests { #[test] fn ambient_hint_with_memo_carries_selected_index() { - use crate::completion::LastCompletion; - // Simulate the post-Tab state at "show " — but with - // the original word still pending (cursor placed - // after `show ` to expose the multi-candidate slot). - // The memo's selection_idx is what the renderer uses - // to highlight one of the items. + use crate::completion::{Candidate, CandidateKind, LastCompletion}; let memo = LastCompletion { inserted_range: (5, 5), original_text: String::new(), - candidates: vec!["data".to_string(), "table".to_string()], + candidates: vec![ + Candidate { text: "data".to_string(), kind: CandidateKind::Keyword }, + Candidate { text: "table".to_string(), kind: CandidateKind::Keyword }, + ], selection_idx: 1, }; match ambient_hint("show ", 5, Some(&memo), &empty_cache()) { Some(AmbientHint::Candidates { items, selected }) => { - assert_eq!(items, vec!["data".to_string(), "table".to_string()]); + assert_eq!(items.len(), 2); + assert_eq!(items[0].text, "data"); + assert_eq!(items[1].text, "table"); assert_eq!(selected, Some(1)); } other => panic!("expected Candidates, got {other:?}"), } } + #[test] + fn ambient_hint_during_cycling_shows_memo_list_not_recomputed() { + // Stage-8 user-reported #4: cycling through candidates + // moves the cursor; the panel should NOT shift to "what + // comes next at the new cursor position" — it should + // keep showing the memo's candidate list with the + // updated selection. Without the memo short-circuit, + // ambient_hint would recompute candidates_at_cursor + // post-Tab and produce a different list. + use crate::completion::{Candidate, CandidateKind, LastCompletion}; + let memo = LastCompletion { + inserted_range: (5, 11), + original_text: String::new(), + // Include candidates whose order would NOT match + // what candidates_at_cursor("show table ", 11) would + // produce — proves the memo's list is being used, + // not a recomputed one. + candidates: vec![ + Candidate { text: "data".to_string(), kind: CandidateKind::Keyword }, + Candidate { text: "table".to_string(), kind: CandidateKind::Keyword }, + ], + selection_idx: 1, + }; + match ambient_hint("show table ", 11, Some(&memo), &empty_cache()) { + Some(AmbientHint::Candidates { items, selected }) => { + assert_eq!(items.len(), 2); + assert_eq!(items[0].text, "data"); + assert_eq!(items[1].text, "table"); + assert_eq!(selected, Some(1)); + } + other => panic!("expected Candidates from memo, got {other:?}"), + } + } + // ---- classify_input + error overlay (stage 4) ---- #[test] diff --git a/src/theme.rs b/src/theme.rs index a51764b..d9c46f5 100644 --- a/src/theme.rs +++ b/src/theme.rs @@ -70,7 +70,7 @@ impl Theme { // accent tones; keyword takes a cool accent tone // distinct from the mode-banner blue. tok_keyword: Color::Rgb(0xC7, 0x92, 0xEA), // muted purple - tok_identifier: Color::Rgb(0xE6, 0xE6, 0xE6), // == fg + tok_identifier: Color::Rgb(0xAB, 0xB2, 0xBF), // cool grey-blue (distinct from fg) tok_number: Color::Rgb(0xF7, 0x8C, 0x6C), // warm orange tok_string: Color::Rgb(0xC3, 0xE8, 0x8D), // soft green tok_punct: Color::Rgb(0x8B, 0x90, 0x9A), // == muted @@ -96,7 +96,7 @@ impl Theme { // identifier/punct close to fg/muted; warm tones for // literals + flags; cool accent for keyword. tok_keyword: Color::Rgb(0x6F, 0x42, 0xC1), // royal purple - tok_identifier: Color::Rgb(0x1A, 0x1F, 0x2C), // == fg + tok_identifier: Color::Rgb(0x3F, 0x47, 0x57), // dark steel-blue (distinct from fg) tok_number: Color::Rgb(0xBC, 0x4F, 0x1F), // burnt orange tok_string: Color::Rgb(0x22, 0x86, 0x3A), // forest green tok_punct: Color::Rgb(0x60, 0x66, 0x73), // == muted diff --git a/src/ui.rs b/src/ui.rs index b999bf8..48d3f57 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -730,16 +730,20 @@ fn render_hint_panel(app: &App, theme: &Theme, frame: &mut Frame<'_>, area: Rect } /// Render the candidate-list line for the hint panel -/// (ADR-0022 §7 + the user's #2). Items are space-separated; -/// the selected item (if any) gets a brighter colour + bold; -/// when the items overflow `width`, a window centred on the -/// selected (or starting at item 0 with no selection) shows -/// scroll markers `<` / `>` at the edges. +/// (ADR-0022 §7 + the user's #2). Items are space-separated. +/// Each candidate is colour-coded by kind — keywords in +/// `tok_keyword`, identifiers in `tok_identifier` — so the +/// user can tell command grammar apart from schema names at +/// a glance (post-stage-8 user feedback). The selected item +/// (if any) gets bolded; when the items overflow `width`, +/// scroll markers `<` / `>` appear at the edges with the +/// window centred on the selection (or item 0 with no +/// selection). /// /// Returns `Line<'static>` (each item cloned into its span) /// so the caller doesn't have to manage the items' lifetime. fn render_candidate_line( - items: &[String], + items: &[crate::completion::Candidate], selected: Option, width: usize, theme: &Theme, @@ -747,29 +751,32 @@ fn render_candidate_line( if items.is_empty() { return Line::default(); } - let item_style = Style::default().fg(theme.muted); - let selected_style = Style::default() - .fg(theme.fg) - .add_modifier(Modifier::BOLD); + let separator_style = Style::default().fg(theme.muted); let marker_style = Style::default().fg(theme.fg); + let style_for = |i: usize| { + let base_fg = match items[i].kind { + crate::completion::CandidateKind::Keyword => theme.tok_keyword, + crate::completion::CandidateKind::Identifier => theme.tok_identifier, + }; + let mut s = Style::default().fg(base_fg); + if Some(i) == selected { + s = s.add_modifier(Modifier::BOLD); + } + s + }; let total_width: usize = items .iter() - .map(|s| s.len() + 1) + .map(|c| c.text.len() + 1) .sum::() .saturating_sub(1); if total_width <= width { let mut spans: Vec> = Vec::with_capacity(items.len() * 2); for (i, item) in items.iter().enumerate() { if i > 0 { - spans.push(Span::styled(" ".to_string(), item_style)); + spans.push(Span::styled(" ".to_string(), separator_style)); } - let style = if Some(i) == selected { - selected_style - } else { - item_style - }; - spans.push(Span::styled(item.clone(), style)); + spans.push(Span::styled(item.text.clone(), style_for(i))); } return Line::from(spans); } @@ -780,12 +787,11 @@ fn render_candidate_line( let center = selected.unwrap_or(0); let mut left = center; let mut right = center; - let mut used = items[center].len(); + let mut used = items[center].text.len(); let avail = width.saturating_sub(4); while (left > 0 || right + 1 < items.len()) && used < avail { - // Expand right first, then left. if right + 1 < items.len() { - let cost = items[right + 1].len() + 1; + let cost = items[right + 1].text.len() + 1; if used + cost <= avail { right += 1; used += cost; @@ -793,7 +799,7 @@ fn render_candidate_line( } } if left > 0 { - let cost = items[left - 1].len() + 1; + let cost = items[left - 1].text.len() + 1; if used + cost <= avail { left -= 1; used += cost; @@ -811,14 +817,9 @@ fn render_candidate_line( } for (offset, item) in items[left..=right].iter().enumerate() { if offset > 0 { - spans.push(Span::styled(" ".to_string(), item_style)); + spans.push(Span::styled(" ".to_string(), separator_style)); } - let style = if Some(left + offset) == selected { - selected_style - } else { - item_style - }; - spans.push(Span::styled(item.clone(), style)); + spans.push(Span::styled(item.text.clone(), style_for(left + offset))); } if need_right_marker { spans.push(Span::styled(" >".to_string(), marker_style));