round-5 follow-up: completion + i18n sweep
Four user-reported gaps from the round-4 testing pass:
1. Empty-prompt hint reworded from "(no active hint)" to
"Type a command — press Tab for options, `help` for a
list" (6 snapshots updated to reflect 80-col truncation).
2. App-lifecycle commands (quit/q, help, rebuild, save/save as,
new, load, export, import, mode, messages) now flow through
the DSL parser:
- 15 new keywords + catalog token entries
- new Command::App(AppCommand) AST with 11 variants
- parse-first dispatch in submit() (app commands work in
both simple and advanced modes)
- pre-chumsky source-slice for `export <path>` /
`import <zip> [as <target>]` mirrors the replay precedent
- UsageEntry registry entries so parse errors surface
relevant usage templates
- `mode <bad>` / `messages <bad>` use try_map for the
friendly "unknown mode/messages" wording
3. DSL completion gaps:
- `1:n` surfaces as a composite candidate at `add `
- --all-rows / --create-fk / --force-conversion /
--dont-convert surface as new CandidateKind::Flag
candidates (coloured with tok_flag in hint panel)
- filter_clause .labelled() wrap removed so chumsky's
expected-set surfaces the constituent options
4. Hardcoded user-facing strings migrated to catalog:
- 4 parser custom errors (incl. the known "tables need at
least one column" wart)
- UnknownType Display now via parse.custom.unknown_type
- UI panel titles + mode labels (Output / Hint / SIMPLE /
ADVANCED / Advanced:)
- app.rs cascade rendering (action labels + summary)
- runtime --resume CLI stderr
- db.rs change-column diagnostic tables (7 headers + 3
wrapper summaries + force-conversion hint)
Tests: 765 → 769 passing, 0 failed, 1 ignored (same doctest
as before). Clippy clean with -D warnings.
Deferred:
- ~25 thiserror #[error] attributes still hand-rolled
(DbError, ArgsError, ArchiveError, PersistenceError,
LockError). Tracked separately.
- DSL/SQL relationship in advanced mode — clarified
implicitly via parse-first dispatch; broader ADR
amendment to follow.
- Post-complete-parse completion gap (e.g. `save ` Tab
can't offer `as` because `save` parses bare; same shape
as `--create-fk` after a complete `add relationship`).
This commit is contained in:
+130
-109
@@ -889,61 +889,17 @@ impl App {
|
||||
return Vec::new();
|
||||
}
|
||||
|
||||
// Canonical app-level commands recognised in both modes.
|
||||
// Track-2's full lifecycle command set lands across
|
||||
// Iterations 4 (rebuild, save, save as, new, load) and
|
||||
// 5 (export, import).
|
||||
match effective_input.as_str() {
|
||||
"quit" | "q" => return vec![Action::Quit],
|
||||
"help" => {
|
||||
self.note_help();
|
||||
return Vec::new();
|
||||
}
|
||||
"rebuild" => return vec![Action::PrepareRebuild],
|
||||
"save" => {
|
||||
return self.handle_save_command(false);
|
||||
}
|
||||
"save as" => {
|
||||
return self.handle_save_command(true);
|
||||
}
|
||||
"new" => {
|
||||
return vec![Action::NewProject {
|
||||
source: "new".to_string(),
|
||||
}];
|
||||
}
|
||||
"load" => {
|
||||
return vec![Action::OpenLoadPicker];
|
||||
}
|
||||
"export" => {
|
||||
return vec![Action::Export {
|
||||
target: None,
|
||||
source: "export".to_string(),
|
||||
}];
|
||||
}
|
||||
other if other.starts_with("export ") => {
|
||||
let target = other["export ".len()..].trim();
|
||||
if target.is_empty() {
|
||||
self.note_error(crate::t!("project.export_usage"));
|
||||
return Vec::new();
|
||||
}
|
||||
return vec![Action::Export {
|
||||
target: Some(target.to_string()),
|
||||
source: format!("export {target}"),
|
||||
}];
|
||||
}
|
||||
other if other.starts_with("import ") || other == "import" => {
|
||||
let rest = other.strip_prefix("import").unwrap_or(other);
|
||||
return self.handle_import_command(rest);
|
||||
}
|
||||
other if other.starts_with("mode") => {
|
||||
self.handle_mode_command(other);
|
||||
return Vec::new();
|
||||
}
|
||||
other if other.starts_with("messages") => {
|
||||
self.handle_messages_command(other);
|
||||
return Vec::new();
|
||||
}
|
||||
_ => {}
|
||||
// Parse-first: app-level commands and DSL commands now
|
||||
// share the chumsky parser (per the round-5 refactor).
|
||||
// App commands work in both modes — they're not gated by
|
||||
// `effective_mode`. Anything that parses to a non-App
|
||||
// variant falls through to the existing mode-specific
|
||||
// path: simple → DSL execution; advanced → SQL placeholder.
|
||||
// Anything that fails to parse falls through too — the
|
||||
// simple-mode path renders the friendly parse error, the
|
||||
// advanced-mode path renders the SQL placeholder.
|
||||
if let Ok(Command::App(app_cmd)) = parse_command(&effective_input) {
|
||||
return self.dispatch_app_command(app_cmd, &effective_input);
|
||||
}
|
||||
|
||||
// For everything else: dispatch by effective mode.
|
||||
@@ -968,6 +924,79 @@ impl App {
|
||||
}
|
||||
}
|
||||
|
||||
/// Dispatch a parsed app-lifecycle command. Works in both
|
||||
/// simple and advanced modes; the parse-first refactor
|
||||
/// (round-5) routes app commands here before the
|
||||
/// mode-specific DSL/SQL paths.
|
||||
fn dispatch_app_command(
|
||||
&mut self,
|
||||
cmd: crate::dsl::AppCommand,
|
||||
source: &str,
|
||||
) -> Vec<Action> {
|
||||
use crate::dsl::{AppCommand, MessagesValue, ModeValue};
|
||||
match cmd {
|
||||
AppCommand::Quit => vec![Action::Quit],
|
||||
AppCommand::Help => {
|
||||
self.note_help();
|
||||
Vec::new()
|
||||
}
|
||||
AppCommand::Rebuild => vec![Action::PrepareRebuild],
|
||||
AppCommand::Save => self.handle_save_command(false),
|
||||
AppCommand::SaveAs => self.handle_save_command(true),
|
||||
AppCommand::New => vec![Action::NewProject {
|
||||
source: "new".to_string(),
|
||||
}],
|
||||
AppCommand::Load => vec![Action::OpenLoadPicker],
|
||||
AppCommand::Export { path } => path.map_or_else(
|
||||
|| {
|
||||
vec![Action::Export {
|
||||
target: None,
|
||||
source: "export".to_string(),
|
||||
}]
|
||||
},
|
||||
|target| {
|
||||
vec![Action::Export {
|
||||
source: format!("export {target}"),
|
||||
target: Some(target),
|
||||
}]
|
||||
},
|
||||
),
|
||||
AppCommand::Import { path, target } => {
|
||||
// The path-bearing import goes through the
|
||||
// pre-chumsky source-slice (parser.rs), which
|
||||
// already validated non-empty path. Bare
|
||||
// `import` returns from chumsky with an empty
|
||||
// path string — surface the usage error.
|
||||
if path.is_empty() {
|
||||
self.note_error(crate::t!("project.import_usage"));
|
||||
return Vec::new();
|
||||
}
|
||||
vec![Action::Import {
|
||||
zip_path: path,
|
||||
as_target: target,
|
||||
source: source.to_string(),
|
||||
}]
|
||||
}
|
||||
AppCommand::Mode { value } => {
|
||||
let arg = match value {
|
||||
ModeValue::Simple => "simple",
|
||||
ModeValue::Advanced => "advanced",
|
||||
};
|
||||
self.handle_mode_command(&format!("mode {arg}"));
|
||||
Vec::new()
|
||||
}
|
||||
AppCommand::Messages { value } => {
|
||||
let raw = match value {
|
||||
None => "messages".to_string(),
|
||||
Some(MessagesValue::Short) => "messages short".to_string(),
|
||||
Some(MessagesValue::Verbose) => "messages verbose".to_string(),
|
||||
};
|
||||
self.handle_messages_command(&raw);
|
||||
Vec::new()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn dispatch_dsl(&mut self, input: &str, submission_mode: Mode) -> Vec<Action> {
|
||||
match parse_command(input) {
|
||||
Ok(Command::Replay { path }) => {
|
||||
@@ -1268,6 +1297,13 @@ impl App {
|
||||
(Operation::Query, Some(name.as_str()), None)
|
||||
}
|
||||
C::Replay { .. } => (Operation::Replay, None, None),
|
||||
// App-lifecycle commands never reach this path —
|
||||
// `dispatch_input` routes them through
|
||||
// `dispatch_app_command` before the DSL execution
|
||||
// pipeline that this context builder feeds.
|
||||
C::App(_) => unreachable!(
|
||||
"App commands are dispatched before reaching dsl execution"
|
||||
),
|
||||
};
|
||||
|
||||
TranslateContext {
|
||||
@@ -1298,42 +1334,6 @@ impl App {
|
||||
/// "as" is fine — the separator only matches when
|
||||
/// surrounded by spaces. `split_once` is used (first
|
||||
/// occurrence wins), which is the natural reading.
|
||||
fn handle_import_command(&mut self, rest: &str) -> Vec<Action> {
|
||||
let rest = rest.trim();
|
||||
if rest.is_empty() {
|
||||
self.note_error(crate::t!("project.import_usage"));
|
||||
return Vec::new();
|
||||
}
|
||||
// `submit()` trims trailing whitespace from the raw
|
||||
// line, so an input like `import foo.zip as ` arrives
|
||||
// here as `foo.zip as`. Detect that explicitly rather
|
||||
// than silently treating "as" as part of the zip
|
||||
// path.
|
||||
if rest == "as" || rest.ends_with(" as") {
|
||||
self.note_error(crate::t!("project.import_empty_target"));
|
||||
return Vec::new();
|
||||
}
|
||||
let (zip_path, as_target) = match rest.split_once(" as ") {
|
||||
Some((zip, target)) => (zip.trim(), Some(target.trim().to_string())),
|
||||
None => (rest, None),
|
||||
};
|
||||
if zip_path.is_empty() {
|
||||
self.note_error(crate::t!("project.import_usage"));
|
||||
return Vec::new();
|
||||
}
|
||||
if let Some(t) = as_target.as_deref()
|
||||
&& t.is_empty()
|
||||
{
|
||||
self.note_error(crate::t!("project.import_empty_target"));
|
||||
return Vec::new();
|
||||
}
|
||||
vec![Action::Import {
|
||||
zip_path: zip_path.to_string(),
|
||||
as_target,
|
||||
source: format!("import {rest}"),
|
||||
}]
|
||||
}
|
||||
|
||||
/// Dispatch for the `save` and `save as` commands.
|
||||
///
|
||||
/// `save` on a temp project is identical to `save as`
|
||||
@@ -1752,18 +1752,20 @@ fn render_usage_block(input: &str, position: usize) -> String {
|
||||
|
||||
fn render_cascade_effect(effect: &CascadeEffect) -> String {
|
||||
use crate::dsl::ReferentialAction;
|
||||
let what = match effect.action {
|
||||
ReferentialAction::Cascade => "deleted",
|
||||
ReferentialAction::SetNull => "had FK set to null",
|
||||
ReferentialAction::Restrict | ReferentialAction::NoAction => "blocked",
|
||||
let action_key = match effect.action {
|
||||
ReferentialAction::Cascade => "db.cascade.action_deleted",
|
||||
ReferentialAction::SetNull => "db.cascade.action_set_null",
|
||||
ReferentialAction::Restrict | ReferentialAction::NoAction => {
|
||||
"db.cascade.action_blocked"
|
||||
}
|
||||
};
|
||||
format!(
|
||||
" related: {} row(s) {} in `{}` for relationship `{}` (on delete {})",
|
||||
effect.rows_changed,
|
||||
what,
|
||||
effect.child_table,
|
||||
effect.relationship_name,
|
||||
effect.action,
|
||||
crate::t!(
|
||||
"db.cascade.summary",
|
||||
count = effect.rows_changed,
|
||||
action = crate::friendly::translate(action_key, &[]),
|
||||
child_table = effect.child_table,
|
||||
rel = effect.relationship_name,
|
||||
on_delete = effect.action,
|
||||
)
|
||||
}
|
||||
|
||||
@@ -1966,13 +1968,17 @@ mod tests {
|
||||
// Stage-8 follow-up #2 (testing-round-2): the
|
||||
// single-candidate-no-memo design lets the user chain
|
||||
// Tabs through unique completions without getting
|
||||
// stuck. From "a", Tab → "add ", Tab → "add column ".
|
||||
// stuck. From "cr", Tab → "create ", Tab → "create
|
||||
// table ". (Round 5 added the app-lifecycle commands —
|
||||
// single-letter prefixes like `i` are now ambiguous
|
||||
// (`insert` vs. `import`), so the test starts from a
|
||||
// disambiguated two-letter prefix.)
|
||||
let mut app = App::new();
|
||||
type_str(&mut app, "a");
|
||||
type_str(&mut app, "cr");
|
||||
app.update(key(KeyCode::Tab));
|
||||
assert_eq!(app.input, "add ");
|
||||
assert_eq!(app.input, "create ");
|
||||
app.update(key(KeyCode::Tab));
|
||||
assert_eq!(app.input, "add column ");
|
||||
assert_eq!(app.input, "create table ");
|
||||
assert!(app.last_completion.is_none());
|
||||
}
|
||||
|
||||
@@ -2102,9 +2108,24 @@ mod tests {
|
||||
type_str(&mut app, "mode sideways");
|
||||
submit(&mut app);
|
||||
assert_eq!(app.mode, Mode::Simple);
|
||||
let last = app.output.back().unwrap();
|
||||
assert_eq!(last.kind, OutputKind::Error);
|
||||
assert!(last.text.contains("unknown mode"));
|
||||
// The error surfaces somewhere in the output buffer
|
||||
// (could be the caret line, the parse-error detail
|
||||
// line, or the usage line). Scan for the friendly
|
||||
// "unknown mode" anchor phrase.
|
||||
let anywhere = app
|
||||
.output
|
||||
.iter()
|
||||
.any(|l| l.text.contains("unknown mode"));
|
||||
assert!(
|
||||
anywhere,
|
||||
"expected 'unknown mode' somewhere in output: {:?}",
|
||||
app.output.iter().map(|l| &l.text).collect::<Vec<_>>(),
|
||||
);
|
||||
let any_error = app
|
||||
.output
|
||||
.iter()
|
||||
.any(|l| l.kind == OutputKind::Error);
|
||||
assert!(any_error, "expected at least one Error line");
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user