feat(CLI): did you mean for struct values

* functionality is cursor-aware, and fixes the actual string the user passed in. That way, it is made very clear how the suggested value is to be used. * it's a known weakness of the implementation that it operates on a flattened list of field names, and thus may make nonsensical suggestions. * added punctuation to all errors Fixes #67 [skip ci]
2026-01-07 03:56:42 +01:00 · 2015-05-02 14:03:35 +02:00
parent d2a4e2ff8b
commit 96415d17ca
6 changed files with 117 additions and 30 deletions
--- a/src/mako/cli/lib/argparse.mako
+++ b/src/mako/cli/lib/argparse.mako
@@ -178,7 +178,7 @@ let arg_data = [
    if mc.request_value:
        args.append((
                STRUCT_FLAG,
-                "Set various fields of the request structure",
+                "Set various fields of the request structure, matching the key=value form",
                KEY_VALUE_ARG,
                True,
                True,
@@ -198,7 +198,7 @@ let arg_data = [
    if mc.optional_props or parameters is not UNDEFINED:
        args.append((
                PARAM_FLAG,
-                "Set various fields of the request structure",
+                "Set various optional parameters, matching the key=value form",
                VALUE_ARG,
                False,
                True,
--- a/src/mako/cli/lib/cli.py
+++ b/src/mako/cli/lib/cli.py
@@ -82,6 +82,10 @@ def new_method_context(resource, method, c):
    return MethodContext(m, response_schema, params, request_value, media_params, 
                         required_props, optional_props, part_prop)

+# Returns a string representing a string-vector of mangled names
+# fields is an iterator
+def field_vec(fields):
+    return "vec![%s]" % ', '.join('"%s"' % mangle_subcommand(f) for f in fields)

 def pretty(n):
    return ' '.join(s.capitalize() for s in mangle_subcommand(n).split('-'))
--- a/src/mako/cli/lib/engine.mako
+++ b/src/mako/cli/lib/engine.mako
@@ -8,7 +8,7 @@
                     call_method_ident, POD_TYPES, opt_value, ident, JSON_TYPE_VALUE_MAP,
                     KEY_VALUE_ARG, to_cli_schema, SchemaEntry, CTYPE_POD, actual_json_type, CTYPE_MAP, CTYPE_ARRAY,
                     application_secret_path, DEBUG_FLAG, DEBUG_AUTH_FLAG, CONFIG_DIR_FLAG, req_value, MODE_ARG, 
-                     opt_values, SCOPE_ARG, CONFIG_DIR_ARG, DEFAULT_MIME)
+                     opt_values, SCOPE_ARG, CONFIG_DIR_ARG, DEFAULT_MIME, field_vec)

    v_arg = '<%s>' % VALUE_ARG
    SOPT = 'self.opt'
@@ -259,7 +259,7 @@ ${value_unwrap}\
            call = call.${ADD_PARAM_FN}(map.iter().find(|t| t.0 == key).unwrap_or(&("", key)).1, ${value_unwrap})
        },
    % endif # handle global parameters
-        _ => err.issues.push(CLIError::UnknownParameter(key.to_string())),
+        _ => err.issues.push(CLIError::UnknownParameter(key.to_string(), ${field_vec(global_parameter_names)})),
    }
 }
 % endif # handle call parameters
@@ -333,7 +333,7 @@ if dry_run {
 <%
    allow_optionals_fn = lambda s: is_schema_with_optionals(schema_markers(s, c, transitive=False))

-    def flatten_schema_fields(schema, res, init_fn_map, cur=list(), init_call=None):
+    def flatten_schema_fields(schema, res, init_fn_map, fields, cur=list(), init_call=None):
        if len(cur) == 0:
            init_call = ''
            cur = list()
@@ -345,6 +345,7 @@ if dry_run {
            opt_access = ''
        for fn, f in schema.fields.iteritems():
            cur.append(['%s%s' % (mangle_ident(fn), opt_access), fn])
+            fields.add(fn)
            if isinstance(f, SchemaEntry):
                cur[-1][0] = mangle_ident(fn)
                res.append((init_call, schema, f, list(cur)))
@@ -367,14 +368,15 @@ if dry_run {
                               
                    init_fn_map[init_fn_name] = init_fn
                # end handle init
-                flatten_schema_fields(f, res, init_fn_map, cur, init_call)
+                flatten_schema_fields(f, res, init_fn_map, fields, cur, init_call)
            cur.pop()
        # endfor
    # end utility

    schema_fields = list()
    init_fn_map = dict()
-    flatten_schema_fields(request_cli_schema, schema_fields, init_fn_map)
+    fields = set()
+    flatten_schema_fields(request_cli_schema, schema_fields, init_fn_map, fields)
 %>\
 let mut ${request_prop_name} = api::${request_prop_type}::default();
 let mut field_cursor = FieldCursor::default();
@@ -443,7 +445,8 @@ ${opt_suffix}\
            },
        % endfor # each nested field
        _ => {
-            err.issues.push(CLIError::Field(FieldError::Unknown(temp_cursor.to_string())));
+            let suggestion = FieldCursor::did_you_mean(key, &${field_vec(sorted(fields))});
+            err.issues.push(CLIError::Field(FieldError::Unknown(temp_cursor.to_string(), suggestion, value.map(|v| v.to_string()))));
        }
    }
 }
--- a/src/mako/cli/main.rs.mako
+++ b/src/mako/cli/main.rs.mako
@@ -21,6 +21,7 @@ extern crate yup_hyper_mock as mock;
 extern crate serde;
 extern crate hyper;
 extern crate mime;
+extern crate strsim;
 extern crate ${to_extern_crate_name(library_to_crate_name(library_name(name, version), make.depends_on_suffix))} as api;

 use std::env;
--- a/src/rust/cli/cmn.rs
+++ b/src/rust/cli/cmn.rs
@@ -2,6 +2,7 @@ use oauth2::{ApplicationSecret, ConsoleApplicationSecret, TokenStorage, Token};
 use serde::json;
 use mime::Mime;
 use clap::{App, SubCommand};
+use strsim;

 use std::fs;
 use std::env;
@@ -16,6 +17,21 @@ use std::default::Default;

 const FIELD_SEP: char = '.';

+fn did_you_mean<'a>(v: &str, possible_values: &[&'a str]) -> Option<&'a str> {
+
+    let mut candidate: Option<(f64, &str)> = None;
+    for pv in possible_values {
+        let confidence = strsim::jaro_winkler(v, pv);
+        if confidence > 0.8 && (candidate.is_none() || (candidate.as_ref().unwrap().0 < confidence)) {
+            candidate = Some((confidence, pv));
+        }
+    }
+    match candidate {
+        None => None,
+        Some((_, candidate)) => Some(candidate),
+    }
+}
+
 pub enum CallType {
    Upload(UploadProtocol),
    Standard,
@@ -127,6 +143,49 @@ impl FieldCursor {
        Ok(())
    }

+    pub fn did_you_mean(value: &str, possible_values: &[&str]) -> Option<String> {
+        if value.len() == 0 {
+            return None
+        }
+
+        let mut last_c = FIELD_SEP;
+
+        let mut field = String::new();
+        let mut output = String::new();
+
+        let push_field = |fs: &mut String, f: &mut String| {
+            if f.len() > 0 {
+                fs.push_str(
+                    match did_you_mean(&f, possible_values) {
+                        Some(candidate) => candidate,
+                        None => &f,
+                    });
+                f.truncate(0);
+            }
+        };
+
+        for (cid, c) in value.chars().enumerate() {
+            if c == FIELD_SEP {
+                if last_c != FIELD_SEP {
+                    push_field(&mut output, &mut field);
+                }
+                output.push(c);
+            } else {
+                field.push(c);
+            }
+
+            last_c = c;
+        }
+
+        push_field(&mut output, &mut field);
+
+        if &output == value {
+            None
+        } else {
+            Some(output)
+        }
+    }
+
    pub fn num_fields(&self) -> usize {
        self.0.len()
    }
@@ -280,10 +339,10 @@ impl fmt::Display for ApplicationSecretError {
    fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
        match *self {
            ApplicationSecretError::DecoderError((ref path, ref err))
-                => writeln!(f, "Could not decode file at '{}' with error: {}", 
+                => writeln!(f, "Could not decode file at '{}' with error: {}.", 
                            path, err),
            ApplicationSecretError::FormatError(ref path)
-                => writeln!(f, "'installed' field is unset in secret file at '{}'", 
+                => writeln!(f, "'installed' field is unset in secret file at '{}'.", 
                            path),
        }
    }
@@ -302,15 +361,15 @@ impl fmt::Display for ConfigurationError {
    fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
        match *self {
            ConfigurationError::DirectoryCreationFailed((ref dir, ref err))
-                => writeln!(f, "Directory '{}' could not be created with error: {}", dir, err),
+                => writeln!(f, "Directory '{}' could not be created with error: {}.", dir, err),
            ConfigurationError::DirectoryUnset 
-                => writeln!(f, "--config-dir was unset or empty"),
+                => writeln!(f, "--config-dir was unset or empty."),
            ConfigurationError::HomeExpansionFailed(ref dir)
-                => writeln!(f, "Couldn't find HOME directory of current user, failed to expand '{}'", dir),
+                => writeln!(f, "Couldn't find HOME directory of current user, failed to expand '{}'.", dir),
            ConfigurationError::Secret(ref err)
                => writeln!(f, "Secret -> {}", err),
            ConfigurationError::IOError((ref path, ref err))
-                => writeln!(f, "IO operation failed on path '{}' with error: {}", path, err),
+                => writeln!(f, "IO operation failed on path '{}' with error: {}.", path, err),
        }
    }
 }
@@ -325,9 +384,9 @@ impl fmt::Display for InputError {
    fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
        match *self {
            InputError::IOError((ref file_path, ref io_err))
-                => writeln!(f, "Failed to open '{}' for reading with error: {}", file_path, io_err),
+                => writeln!(f, "Failed to open '{}' for reading with error: {}.", file_path, io_err),
            InputError::Mime(ref mime)
-                => writeln!(f, "'{}' is not a known mime-type", mime),
+                => writeln!(f, "'{}' is not a known mime-type.", mime),
        }
    }
 }
@@ -336,7 +395,7 @@ impl fmt::Display for InputError {
 pub enum FieldError {
    PopOnEmpty(String),
    TrailingFieldSep(String),
-    Unknown(String),
+    Unknown(String, Option<String>, Option<String>),
    Empty,
 }

@@ -345,13 +404,26 @@ impl fmt::Display for FieldError {
    fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
        match *self {
            FieldError::PopOnEmpty(ref field)
-                => writeln!(f, "'{}': Cannot move up on empty field cursor", field),
+                => writeln!(f, "'{}': Cannot move up on empty field cursor.", field),
            FieldError::TrailingFieldSep(ref field)
-                => writeln!(f, "'{}': Single field separator may not be last character", field),
-            FieldError::Unknown(ref field)
-                => writeln!(f, "Field '{}' does not exist", field),
+                => writeln!(f, "'{}': Single field separator may not be last character.", field),
+            FieldError::Unknown(ref field, ref suggestion, ref value) => {
+                let suffix = 
+                    match *suggestion {
+                        Some(ref s) => {
+                            let kv = 
+                                match *value {
+                                    Some(ref v) => format!("{}={}", s, v),
+                                    None => s.clone(),
+                                };
+                            format!(" Did you mean '{}' ?", kv)
+                        },
+                        None => String::new(),
+                    };
+                writeln!(f, "Field '{}' does not exist.{}", field, suffix)
+            },
            FieldError::Empty
-                => writeln!(f, "Field names must not be empty"),
+                => writeln!(f, "Field names must not be empty."),
        }
    }
 }
@@ -361,7 +433,7 @@ impl fmt::Display for FieldError {
 pub enum CLIError {
    Configuration(ConfigurationError),
    ParseError(&'static str, &'static str, String, String),
-    UnknownParameter(String),
+    UnknownParameter(String, Vec<&'static str>),
    InvalidUploadProtocol(String, Vec<String>),
    InvalidKeyValueSyntax(String, bool),
    Input(InputError),
@@ -377,18 +449,24 @@ impl fmt::Display for CLIError {
            CLIError::Input(ref err) => write!(f, "Input -> {}", err),
            CLIError::Field(ref err) => write!(f, "Field -> {}", err),
            CLIError::InvalidUploadProtocol(ref proto_name, ref valid_names) 
-                => writeln!(f, "'{}' is not a valid upload protocol. Choose from one of {}", proto_name, valid_names.connect(", ")),
+                => writeln!(f, "'{}' is not a valid upload protocol. Choose from one of {}.", proto_name, valid_names.connect(", ")),
            CLIError::ParseError(arg_name, type_name, ref value, ref err_desc) 
-                => writeln!(f, "Failed to parse argument '{}' with value '{}' as {} with error: {}",
+                => writeln!(f, "Failed to parse argument '{}' with value '{}' as {} with error: {}.",
                            arg_name, value, type_name, err_desc),
-            CLIError::UnknownParameter(ref param_name) 
-                => writeln!(f, "Parameter '{}' is unknown.", param_name),
+            CLIError::UnknownParameter(ref param_name, ref possible_values) => {
+                let mut suffix = 
+                    match did_you_mean(param_name, &possible_values) {
+                        Some(v) => format!(" Did you mean '{}' ?", v),
+                        None => String::new(),
+                    };
+                write!(f, "Parameter '{}' is unknown.{}\n", param_name, suffix)
+            },
            CLIError::InvalidKeyValueSyntax(ref kv, is_hashmap) => {
                let hashmap_info = if is_hashmap { "hashmap " } else { "" };
-                writeln!(f, "'{}' does not match {}pattern <key>=<value>", kv, hashmap_info)
+                writeln!(f, "'{}' does not match {}pattern <key>=<value>.", kv, hashmap_info)
            },
-            CLIError::MissingCommandError => writeln!(f, "Please specify the main sub-command"),
-            CLIError::MissingMethodError(ref cmd) => writeln!(f, "Please specify the method to call on the '{}' command", cmd),
+            CLIError::MissingCommandError => writeln!(f, "Please specify the main sub-command."),
+            CLIError::MissingMethodError(ref cmd) => writeln!(f, "Please specify the method to call on the '{}' command.", cmd),
        }
    }
 }