ADR-0022 follow-up r3: identifier colour, NewName hint, "Next:" wording, "type" label
Three fixes from a third round of real testing. 1. **tok_identifier vivid (round-3 #1).** The cool grey-blue from r2 was still too close to theme.fg to register as distinct. Bumped to cyan-teal (#56B6C2 dark / #0F6B76 light) — identifiers are the user's most "special" content and now read that way against keywords (purple), numbers (orange), strings (green), and flags (amber). 2. **"Type a name" hint at NewName slots (round-3 #2).** New `completion::typing_name_at_cursor(input, cursor)` returns `Some(TypingName)` when the cursor sits at — or inside — an `IdentSlot::NewName` position. It probes by substituting a single-letter placeholder identifier and re-parsing to discover what the parser would expect AFTER the name; the hint then reads "Type a name, then `(`" instead of the technical "next: `(`" that surfaces once the partial identifier has been consumed by the live parser. When the probe yields nothing useful (custom errors with empty expected, or a complete-on-substitute case), falls back to "Type a name". New catalog keys hint.ambient_typing_name and hint.ambient_typing_name_then. Wired into ambient_hint between the candidate-list and invalid-ident checks. 3. **"Next:" instead of "expected:" wording.** "Expected" read as a leaked diagnostic; "Next:" is shorter, conversational, and consistent with the action-oriented voice of "Submit with Enter" and "Type a name". Hint sentences now also start capitalised (Submit/Next/Type/No-such), per the user's Capital-T-on- "type a name" preference. 4. **type_keyword labelled "type".** Without a label, the `select_ref!` over an Identifier token produced `RichPattern::SomethingElse`, which rendered as the meaningless "something else" in the hint after `(`. Labelled now: error reads "Next: type" — terse but honest. The label is applied BEFORE try_map (not after, not via as_context) so the existing custom-error wording for unknown types ("unknown type 'varchar' (expected one of: …)") still surfaces unchanged. Tests: 755 passing, 0 failing, 1 ignored (no net change — +5 typing_name cases, -0 net since one test was reworded for capitalisation rather than added). Clippy clean. Smoke probe verifies: "add column to table T: " → "Type a name, then `(`"; "add column to table T: Name (" → "Next: type"; "show data Custp" → "No such table: `Custp`"; valid input → "Submit with Enter". Note for next testing round: parser-side custom errors (e.g. the "tables need at least one column" message that fires for `create table Customers `) still read in lowercase — they're hand-written in parser.rs source rather than via the catalog. If the lowercase "tables need…" intrusion bothers you, easy follow-up.
This commit is contained in:
@@ -199,6 +199,89 @@ pub struct InvalidIdent {
|
||||
pub slot: IdentSlot,
|
||||
}
|
||||
|
||||
/// "User is typing a name" cursor state (round-3 follow-up).
|
||||
///
|
||||
/// Fires at `NewName` slots — positions where the user is
|
||||
/// expected to invent a name (new table, new column, new
|
||||
/// relationship). Used by the hint panel to surface a friendly
|
||||
/// "Type a name" hint instead of the technical "next: `(`"
|
||||
/// that would otherwise appear once the partial identifier
|
||||
/// gets consumed by the parser.
|
||||
///
|
||||
/// `next_after_name` is what the parser would expect once the
|
||||
/// user finishes typing the name — derived by re-parsing with
|
||||
/// a single-letter placeholder identifier substituted at the
|
||||
/// cursor. `None` when the post-name parse succeeds (the rest
|
||||
/// of the command is already in place) or has no meaningful
|
||||
/// next-token information (custom errors with empty expected
|
||||
/// set).
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct TypingName {
|
||||
pub next_after_name: Option<String>,
|
||||
}
|
||||
|
||||
/// `Some(_)` when the cursor is at or inside a `NewName`-slot
|
||||
/// position. Otherwise `None`.
|
||||
#[must_use]
|
||||
pub fn typing_name_at_cursor(input: &str, cursor: usize) -> Option<TypingName> {
|
||||
let cursor = cursor.min(input.len());
|
||||
let bytes = input.as_bytes();
|
||||
let mut start = cursor;
|
||||
while start > 0 {
|
||||
let prev = bytes[start - 1];
|
||||
if prev.is_ascii_alphanumeric() || prev == b'_' {
|
||||
start -= 1;
|
||||
} else {
|
||||
break;
|
||||
}
|
||||
}
|
||||
let leading = &input[..start];
|
||||
let expected = expected_set(leading);
|
||||
let is_new_name_slot = expected
|
||||
.iter()
|
||||
.filter_map(|item| IdentSlot::from_expected_label(item))
|
||||
.any(|slot| slot == IdentSlot::NewName);
|
||||
if !is_new_name_slot {
|
||||
return None;
|
||||
}
|
||||
// Probe what comes after the name by substituting a
|
||||
// single-letter identifier placeholder. Walk forward over
|
||||
// any partial text past the cursor first so the probe
|
||||
// replaces the user's in-progress name as a whole.
|
||||
let mut end = cursor;
|
||||
while end < bytes.len() {
|
||||
let c = bytes[end];
|
||||
if c.is_ascii_alphanumeric() || c == b'_' {
|
||||
end += 1;
|
||||
} else {
|
||||
break;
|
||||
}
|
||||
}
|
||||
let probe = format!("{}X{}", &input[..start], &input[end..]);
|
||||
let next_after_name = match parse_command(&probe) {
|
||||
Ok(_) => None,
|
||||
Err(ParseError::Empty) => None,
|
||||
Err(ParseError::Invalid { expected, .. }) if expected.is_empty() => None,
|
||||
Err(ParseError::Invalid { expected, .. }) => Some(oxford_or(&expected)),
|
||||
};
|
||||
Some(TypingName { next_after_name })
|
||||
}
|
||||
|
||||
/// English-style "A, B, or C" join used by the hint panel
|
||||
/// prose. Lifted out of `input_render` so the completion
|
||||
/// module can produce ready-to-render strings.
|
||||
fn oxford_or(items: &[String]) -> String {
|
||||
match items {
|
||||
[] => String::new(),
|
||||
[a] => a.clone(),
|
||||
[a, b] => format!("{a} or {b}"),
|
||||
rest => {
|
||||
let (last, head) = rest.split_last().expect("len >= 3");
|
||||
format!("{}, or {last}", head.join(", "))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Detect "the user has typed an identifier here that the
|
||||
/// schema doesn't have." Returns `None` for any of:
|
||||
/// - cursor at empty / whitespace partial;
|
||||
@@ -554,6 +637,54 @@ mod tests {
|
||||
assert!(cs.is_empty(), "got {cs:?}");
|
||||
}
|
||||
|
||||
// ---- typing_name_at_cursor (round-3 follow-up) ----
|
||||
|
||||
#[test]
|
||||
fn typing_name_fires_at_new_column_slot_with_next_token() {
|
||||
// After `add column to table T: ` the parser expects
|
||||
// an identifier (NewName slot) followed by `(`. The
|
||||
// probe substitutes a placeholder name and reads back
|
||||
// that the next token is `(`.
|
||||
let t = typing_name_at_cursor("add column to table T: ", 23)
|
||||
.expect("should fire at NewName slot");
|
||||
assert_eq!(t.next_after_name.as_deref(), Some("`(`"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn typing_name_fires_when_partial_name_already_typed() {
|
||||
// Mid-typing the column name. typing_name_at_cursor
|
||||
// walks back over the partial to find the slot, then
|
||||
// probes forward as if the partial were a complete name.
|
||||
let t = typing_name_at_cursor("add column to table T: Na", 25)
|
||||
.expect("should fire at NewName slot with partial");
|
||||
assert_eq!(t.next_after_name.as_deref(), Some("`(`"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn typing_name_does_not_fire_at_table_name_slot() {
|
||||
// `show data ` — the slot is TableName, not NewName.
|
||||
// The candidates path (or invalid-ident) handles it;
|
||||
// typing_name should not fire.
|
||||
assert!(typing_name_at_cursor("show data ", 10).is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn typing_name_does_not_fire_at_keyword_slot() {
|
||||
// `cr` at position 2 is a keyword slot.
|
||||
assert!(typing_name_at_cursor("cr", 2).is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn typing_name_yields_no_next_when_probe_succeeds() {
|
||||
// `add column to table T: Name (text)` — the user is
|
||||
// inside `Name`, and substituting any name there
|
||||
// produces a complete command. No useful "next after
|
||||
// name" hint.
|
||||
let t = typing_name_at_cursor("add column to table T: Name (text)", 27)
|
||||
.expect("should fire");
|
||||
assert_eq!(t.next_after_name, None);
|
||||
}
|
||||
|
||||
// ---- invalid_ident_at_cursor (stage 8e) ----
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -252,11 +252,21 @@ fn flag<'a>(
|
||||
/// existing "unknown type 'X' (expected one of: …)" message
|
||||
/// (ADR-0020 §4) — keyword-shape errors aggregate naturally,
|
||||
/// content errors keep their hand-written voice.
|
||||
///
|
||||
/// Labelled "type" so the structural-error wording reads as
|
||||
/// "next: type" rather than the unhelpful "something else"
|
||||
/// the unlabelled `select_ref!` would otherwise produce.
|
||||
fn type_keyword<'a>()
|
||||
-> impl Parser<'a, &'a [Token], Type, extra::Err<Rich<'a, Token>>> + Clone {
|
||||
// Label is applied to the select-ref alone (before
|
||||
// try_map) so the unknown-type custom error from try_map
|
||||
// still surfaces — labelled() on the whole chain would
|
||||
// replace it with "expected type" and lose the
|
||||
// "unknown type 'X' (expected one of: …)" wording.
|
||||
select_ref! {
|
||||
Token { kind: TokenKind::Identifier(s), .. } = e => (s.clone(), e.span())
|
||||
}
|
||||
.labelled("type")
|
||||
.try_map(|(name, span): (String, SimpleSpan), _| {
|
||||
name.parse::<Type>()
|
||||
.map_err(|err| Rich::custom(span, err.to_string()))
|
||||
|
||||
@@ -132,6 +132,11 @@ pub const KEYS_AND_PLACEHOLDERS: &[(&str, &[&str])] = &[
|
||||
"hint.ambient_invalid_ident",
|
||||
&["kind", "found"],
|
||||
),
|
||||
("hint.ambient_typing_name", &[]),
|
||||
(
|
||||
"hint.ambient_typing_name_then",
|
||||
&["next"],
|
||||
),
|
||||
// ---- Parse error rendering ----
|
||||
("parse.available_commands", &["commands"]),
|
||||
("parse.caret", &["padding"]),
|
||||
|
||||
@@ -255,13 +255,24 @@ hint:
|
||||
# The hint panel goes ambient as soon as the user types
|
||||
# anything — empty input keeps the existing
|
||||
# `panel.hint_empty` content.
|
||||
ambient_complete: "submit with Enter"
|
||||
ambient_expected: "expected: {expected}"
|
||||
# Hint sentences are full standalone phrases, capitalised
|
||||
# at the start. Inline `{message}` substitutions inherit
|
||||
# whatever case the source produced (parser errors,
|
||||
# engine messages) — they're embedded mid-sentence so they
|
||||
# stay lowercase by convention.
|
||||
ambient_complete: "Submit with Enter"
|
||||
ambient_expected: "Next: {expected}"
|
||||
ambient_error_with_usage: "{message} — usage: {usage}"
|
||||
# Invalid identifier in a schema slot (ADR-0022 stage 8e
|
||||
# + the user's #5). Voice mirrors ADR-0019's "no such
|
||||
# {kind}" wording for consistency with engine errors.
|
||||
ambient_invalid_ident: "no such {kind}: `{found}`"
|
||||
ambient_invalid_ident: "No such {kind}: `{found}`"
|
||||
# User-invented-name slot (NewName per IdentSlot). The
|
||||
# probe-derived `{next}` is what comes after the name —
|
||||
# e.g. `(` after a new column name. Empty/unknown `next`
|
||||
# falls through to `ambient_typing_name` instead.
|
||||
ambient_typing_name: "Type a name"
|
||||
ambient_typing_name_then: "Type a name, then {next}"
|
||||
|
||||
parse:
|
||||
# Wrapper around chumsky's structural error message. The
|
||||
|
||||
+13
-2
@@ -188,6 +188,17 @@ pub fn ambient_hint(
|
||||
selected: None,
|
||||
});
|
||||
}
|
||||
// User typing into a NewName slot — show the friendlier
|
||||
// "type a name" hint rather than the technical "next: …"
|
||||
// that the post-consumed-partial parse would otherwise
|
||||
// produce (round-3 follow-up).
|
||||
if let Some(t) = crate::completion::typing_name_at_cursor(input, cursor) {
|
||||
let text = t.next_after_name.map_or_else(
|
||||
|| crate::t!("hint.ambient_typing_name"),
|
||||
|next| crate::t!("hint.ambient_typing_name_then", next = next),
|
||||
);
|
||||
return Some(AmbientHint::Prose(text));
|
||||
}
|
||||
// Invalid identifier: cursor sits in a known-set slot but
|
||||
// the typed prefix matches nothing in the schema. (Stage
|
||||
// 8e / the user's #5.)
|
||||
@@ -567,8 +578,8 @@ mod tests {
|
||||
match ambient_hint("show data Custp", 15, None, &cache) {
|
||||
Some(AmbientHint::Prose(p)) => {
|
||||
assert!(
|
||||
p.contains("no such table"),
|
||||
"expected 'no such table' wording, got {p:?}",
|
||||
p.contains("No such table"),
|
||||
"expected 'No such table' wording, got {p:?}",
|
||||
);
|
||||
assert!(p.contains("Custp"), "should name the bad ident, got {p:?}");
|
||||
}
|
||||
|
||||
+2
-2
@@ -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(0xAB, 0xB2, 0xBF), // cool grey-blue (distinct from fg)
|
||||
tok_identifier: Color::Rgb(0x56, 0xB6, 0xC2), // cyan-teal — identifiers are the user's content, deserve a vivid distinct colour
|
||||
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(0x3F, 0x47, 0x57), // dark steel-blue (distinct from fg)
|
||||
tok_identifier: Color::Rgb(0x0F, 0x6B, 0x76), // deep teal — same role as dark variant: identifiers stand out
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user