oli/rdbms-playground
1
0
Fork 0
Code Issues 4 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix(input): thread the : one-shot escape into live SQL feedback

Browse Source 
The `:` one-shot escape (ADR-0003) is stripped at submission, but the
*live* feedback kept the leading `:` in the buffer it handed the
walker — so Tab completion, the validity verdict, and the highlight
overlays all bailed at the `:` and treated the SQL as an unknown
command. Effect: in `:`-mode, Tab completed nothing and a valid query
could flash an error, while the identical line in full `mode advanced`
worked. (The ambient hint already stripped it, which is why the hint
showed the right column name while Tab did nothing.)

Add `App::feedback_view()` — the `:`-stripped SQL view, the cursor
mapped into it, and the stripped byte offset — and route all four live
paths through it:

- completion (Tab): complete against the view, then shift the returned
  `replaced_range` back by the offset so the edit lands in the buffer;
- validity verdict: verdict the SQL, not the sigil;
- highlight/overlays: new `render_input_runs_feedback` highlights and
  diagnoses the view (shifted by the offset) while the `:` renders as
  plain text;
- ambient hint: consolidated onto `feedback_view`, replacing the
  duplicate local `strip_one_shot_prefix`.
This commit is contained in:
claude@clouddev1
2026-06-12 12:43:00 +00:00
parent 4cacb8261c
commit f7155ceafc
3 changed files with 254 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/app.rs
+140 -19
View File
@@ -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<crate::completion::Completion> {
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 "<text> " (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();
src/input_render.rs
+95 -7
View File
@@ -84,16 +84,60 @@ pub fn render_input_runs_in_mode(
cache: &crate::completion::SchemaCache,
mode: Mode,
) -> Vec<StyledRun> {
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<StyledRun> {
// 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<StyledRun> = 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();
src/ui.rs
+19 -27
View File
@@ -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,