Skip to content

Commit 76841da

Browse files
committed
fix(CLI): optimze argument handling and conversion
* Thanks to a generic function, we save a lot of code within main.rs * more effcient signature for ParseError Fixes #65
1 parent e34e24e commit 76841da

2 files changed

Lines changed: 38 additions & 25 deletions

File tree

src/mako/cli/lib/engine.mako

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,15 @@
66
cmd_ident, call_method_ident, arg_ident, POD_TYPES)
77
88
v_arg = '<%s>' % VALUE_ARG
9+
def to_opt_ident(p):
10+
return 'self.opt.' + arg_ident(p.name)
911
%>\
1012
<%def name="new(c)">\
1113
<%
1214
hub_type_name = 'api::' + hub_type(c.schemas, util.canonical_name())
1315
%>\
1416
mod cmn;
15-
use cmn::{InvalidOptionsError, CLIError, JsonTokenStorage};
17+
use cmn::{InvalidOptionsError, CLIError, JsonTokenStorage, arg_from_str};
1618
use std::default::Default;
1719
use std::str::FromStr;
1820
@@ -126,35 +128,26 @@ self.opt.${cmd_ident(method)} {
126128
<%
127129
prop_name = mangle_ident(p.name)
128130
prop_type = activity_rust_type(c.schemas, p, allow_optionals=False)
129-
opt_ident = 'self.opt.' + arg_ident(p.name)
131+
opt_ident = to_opt_ident(p)
130132
%>\
131133
% if is_request_value_property(mc, p):
132134
let ${prop_name}: api::${prop_type} = Default::default();
133-
% else:
134-
let ${prop_name}: ${prop_type} = \
135-
% if p.type == 'string':
136-
${opt_ident}.clone();
137-
% else:
138-
139-
match FromStr::from_str(&${opt_ident}) {
140-
Err(perr) => {
141-
err.issues.push(CLIError::ParseError(format!("Failed to parse argument <${mangle_subcommand(p.name)}> as ${p.type} with error: {}", perr)));
142-
Default::default()
143-
},
144-
Ok(v) => v,
145-
};
146-
% endif # handle argument type
135+
% elif p.type != 'string':
136+
let ${prop_name}: ${prop_type} = arg_from_str(&${opt_ident}, err, "<${mangle_subcommand(p.name)}>", "${p.type}");
147137
% endif # handle request value
148138
% endfor # each required parameter
149139
<%
150140
call_args = list()
151141
for p in mc.required_props:
152142
borrow = ''
153143
# if type is not available, we know it's the request value, which should also be borrowed
154-
ptype = p.get('type', 'string')
155-
if ptype not in POD_TYPES or ptype == 'string':
144+
ptype = p.get('type', None)
145+
if ptype not in POD_TYPES or ptype in ('string', None):
156146
borrow = '&'
157-
call_args.append(borrow + mangle_ident(p.name))
147+
arg_name = mangle_ident(p.name)
148+
if ptype == 'string':
149+
arg_name = to_opt_ident(p)
150+
call_args.append(borrow + arg_name)
158151
# end for each required prop
159152
%>\
160153
let call = self.hub.${mangle_ident(resource)}().${mangle_ident(method)}(${', '.join(call_args)});
@@ -163,6 +156,7 @@ let call = self.hub.${mangle_ident(resource)}().${mangle_ident(method)}(${', '.j
163156
if dry_run {
164157
None
165158
} else {
159+
assert!(err.issues.len() == 0);
166160
## Make the call, handle uploads, handle downloads (also media downloads|json decoding)
167161
## TODO: unify error handling
168162
% if mc.media_params:

src/rust/cli/cmn.rs

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,29 @@ use std::env;
66
use std::io;
77
use std::fmt;
88
use std::path::{Path, PathBuf};
9+
use std::str::FromStr;
910

1011
use std::io::{Write, Read};
1112

1213
use std::default::Default;
1314

1415

16+
pub fn arg_from_str<T>(arg: &str, err: &mut InvalidOptionsError,
17+
arg_name: &'static str,
18+
arg_type: &'static str) -> T
19+
where T: FromStr + Default,
20+
<T as FromStr>::Err: fmt::Display {
21+
match FromStr::from_str(arg) {
22+
Err(perr) => {
23+
err.issues.push(
24+
CLIError::ParseError((arg_name, arg_type, format!("{}", perr)))
25+
);
26+
Default::default()
27+
},
28+
Ok(v) => v,
29+
}
30+
}
31+
1532
pub struct JsonTokenStorage {
1633
pub program_name: &'static str,
1734
pub db_dir: String,
@@ -57,11 +74,11 @@ impl fmt::Display for ApplicationSecretError {
5774
fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
5875
match *self {
5976
ApplicationSecretError::DecoderError((ref path, ref err))
60-
=> writeln!(f, "Could not decode file at '{}' with error: {}"
61-
, path, err),
77+
=> writeln!(f, "Could not decode file at '{}' with error: {}",
78+
path, err),
6279
ApplicationSecretError::FormatError(ref path)
63-
=> writeln!(f, "'installed' field is unset in secret file at '{}'"
64-
, path),
80+
=> writeln!(f, "'installed' field is unset in secret file at '{}'",
81+
path),
6582
}
6683
}
6784
}
@@ -95,14 +112,16 @@ impl fmt::Display for ConfigurationError {
95112
#[derive(Debug)]
96113
pub enum CLIError {
97114
Configuration(ConfigurationError),
98-
ParseError(String),
115+
ParseError((&'static str, &'static str, String)),
99116
}
100117

101118
impl fmt::Display for CLIError {
102119
fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
103120
match *self {
104121
CLIError::Configuration(ref err) => writeln!(f, "Configuration -> {}", err),
105-
CLIError::ParseError(ref desc) => desc.fmt(f),
122+
CLIError::ParseError((arg_name, type_name, ref err_desc))
123+
=> writeln!(f, "Failed to parse argument {} as {} with error: {}",
124+
arg_name, type_name, err_desc),
106125
}
107126
}
108127
}

0 commit comments

Comments
 (0)