ADR-0022 stage 8 follow-up: fixes from real-app testing
Three fixes from the user's testing run, plus an investigation note on a fourth. #4 Sticky hint during cycling. The previous code recomputed candidates_at_cursor at the post-Tab cursor position, which made the panel whiplash through "what comes next at the new cursor" between cycles. ambient_hint now short-circuits to the memo's stored candidate list while the memo is alive — so Tab Tab Tab keeps showing the same list with the selection moving, then snaps to the post-Tab ambient state once any non-Tab key clears the memo. #2 Candidate ordering and kind-coloured rendering. New `Candidate { text, kind: Keyword|Identifier }` carries the classification through completion, last-completion memo, and ambient-hint payload. candidates_at_cursor now sorts keywords first (alphabetical), identifiers second (alphabetical), and the hint-panel renderer colours keywords in `tok_keyword` and identifiers in `tok_identifier`. Keyword-vs-identifier name collisions resolve in favour of the keyword (rare; the user can still address their table via different syntax). #3 tok_identifier no longer matches theme.fg. Identifiers in the input pane now render in a distinct cool grey-blue (dark) / dark steel-blue (light), so they stand out from prose-like default text without competing with keyword purple. Same colour drives the identifier candidates in the hint panel for visual consistency input ↔ hint. Limitation worth knowing: "keywords first, alphabetical" is not the same as grammatical order. For "add column " the hint shows `table to` not `to table` — chumsky's expected-set doesn't preserve combinator-source order, and encoding it in the registry adds maintenance overhead the fix doesn't cleanly justify. Marked for future revisit if it bites. #1 (Tab does nothing on "add column ") — not reproduced through App::update. The internal logic works correctly: "add column " + Tab inserts "Customers ", second Tab cycles to "Orders ", third to "Thing ". The most likely explanation is a stale binary or a terminal-level event intercept (tmux focus, kitty-keyboard protocol differences, etc.) — needs user verification with a fresh build. Tests: 747 passing, 0 failing, 1 ignored (744 baseline → +3: 2 new completion-ordering cases including the keyword-wins-on-name-collision edge, plus 1 hint-mid-cycle sticky test). Clippy clean.
This commit is contained in:
+131
-27
@@ -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<String>,
|
||||
/// Fitting candidates, ordered keywords-first then
|
||||
/// identifiers, alphabetised within each group, deduplicated.
|
||||
pub candidates: Vec<Candidate>,
|
||||
}
|
||||
|
||||
/// 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<String> = 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<String> = 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<String> = 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<Candidate> = 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<String>,
|
||||
/// 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<Candidate>,
|
||||
/// Which `candidates[i]` is currently visible.
|
||||
pub selection_idx: usize,
|
||||
}
|
||||
@@ -280,12 +320,25 @@ mod tests {
|
||||
|
||||
fn cands(input: &str, cursor: usize) -> Vec<String> {
|
||||
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<String> {
|
||||
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);
|
||||
|
||||
Reference in New Issue
Block a user