fix(clap): various fixes and improvements

* `--version` now includes the API revision we embody
  (using crate_version())
* Allow multiple scopes to be specified, instead of just one. Previously
  this was problemantic due to argument parsing of docopt being greedy.
  However, this also means we have to specify the `-r` flag for each
  invocation. See clap-rs/clap#89 .
* Adapted to new signature of `Arg::possible_values()` and used the
  previously orphaned `UploadProtocol` enum.
* Deduplicated code a little by adding the new `opt_values()` generator
  function.

 Related to #81

src/mako/cli/lib/argparse.mako

@@ -5,7 +5,8 @@
     from util import (put_and, supports_scopes, api_index, indent_by, enclose_in)
     from cli import (mangle_subcommand, new_method_context, PARAM_FLAG, STRUCT_FLAG, UPLOAD_FLAG, OUTPUT_FLAG, VALUE_ARG,
                      CONFIG_DIR, SCOPE_FLAG, is_request_value_property, FIELD_SEP, docopt_mode, FILE_ARG, MIME_ARG, OUT_ARG, 
-                     CONFIG_DIR_FLAG, KEY_VALUE_ARG, to_docopt_arg, DEBUG_FLAG, DEBUG_AUTH_FLAG, MODE_ARG)
+                     CONFIG_DIR_FLAG, KEY_VALUE_ARG, to_docopt_arg, DEBUG_FLAG, DEBUG_AUTH_FLAG, MODE_ARG, SCOPE_ARG, 
+                     CONFIG_DIR_ARG)
 
     def rust_boolean(v):
         return v and 'true' or 'false'
@@ -18,7 +19,7 @@
         elif isinstance(v, basestring):
             v = '"%s"' % v
         elif isinstance(v, list):
-            v = 'vec![%s]' % ','.join('"%s"' % p for p in v)
+            v = 'vec![%s]' % ','.join('UploadProtocol::%s' % p.capitalize() for p in v)
         return 'Some(%s)' % v
 %>\
 <%def name="grammar(c)">\
@@ -62,12 +63,12 @@ ${cargo.doc_base_url + '/' + os.path.dirname(api_index(cargo.doc_base_url, name,
 
 Configuration:
 % if supports_scopes(auth):
-  --${SCOPE_FLAG} <url>  
+  --${SCOPE_FLAG} <${SCOPE_ARG}>...
             Specify the authentication a method should be executed in. Each scope 
             requires the user to grant this application permission to use it. 
             If unset, it defaults to the shortest scope url for a particular method.
 % endif scopes
-  --${CONFIG_DIR_FLAG} <folder>
+  --${CONFIG_DIR_FLAG} <${CONFIG_DIR_ARG}>
             A directory into which we will store our persistent data. Defaults to 
             a user-writable directory that we will create during the first invocation.
             [default: ${CONFIG_DIR}]
@@ -90,59 +91,43 @@ Configuration:
     # (0) = long name
     # (1) = description
     # (2) = argument name, no argument if no argument
+    # (3) = multiple
     global_args = list()
     if supports_scopes(auth):
         global_args.append((
             SCOPE_FLAG,
             "Specify the authentication a method should be executed in. Each scope "
             "requires the user to grant this application permission to use it."
             "If unset, it defaults to the shortest scope url for a particular method.",
-            'url'
+            SCOPE_ARG,
+            True
         ))
     # end add scope arg
     global_args.append((
         CONFIG_DIR_FLAG,
         "A directory into which we will store our persistent data. Defaults to "
         "a user-writable directory that we will create during the first invocation."
         "[default: %s" % CONFIG_DIR,
-        'folder',
+        CONFIG_DIR_ARG,
+        False,
     ))
 
     global_args.append((
         DEBUG_FLAG,  
         "Output all server communication to standard error. `tx` and `rx` are placed "
         "into the same stream.",
-        None
+        None,
+        False,
     ))
 
     global_args.append((
         DEBUG_AUTH_FLAG,
         "Output all communication related to authentication to standard error. `tx` "
         "and `rx` are placed into the same stream.",
-        None
+        None,
+        False,
     ))
 %>\
-use cmn::UploadProtocol;
-
-let mut app = App::new("${util.program_name()}")
-<%block filter="indent_by(7)">\
-.author("${', '.join(cargo.authors)}")
-.version("${cargo.build_version}")
-% if description is not UNDEFINED:
-.about("${description}")
-% endif
-.after_help("${url_info}")
-% for flag, desc, arg_name in global_args:
-.arg(Arg::with_name("${arg_name or flag}")
-        .long("${flag}")
-        .help("${desc}")
-        .takes_value(${rust_boolean(arg_name)}))\
-% if loop.last:
-;
-% else:
-
-% endif
-% endfor
 let arg_data = [
 % for resource in sorted(c.rta_map.keys()):
 <%block filter="indent_by(4)">\
@@ -246,6 +231,27 @@ let arg_data = [
 % endfor # end for each resource
 ];
 
+let mut app = App::new("${util.program_name()}")
+<%block filter="indent_by(7)">\
+.author("${', '.join(cargo.authors)}")
+.version("${util.crate_version()}")
+% if description is not UNDEFINED:
+.about("${description}")
+% endif
+.after_help("${url_info}")
+% for flag, desc, arg_name, multiple in global_args:
+.arg(Arg::with_name("${arg_name or flag}")
+        .long("${flag}")
+        .help("${desc}")
+        .multiple(${rust_boolean(multiple)})
+        .takes_value(${rust_boolean(arg_name)}))\
+% if loop.last:
+;
+% else:
+
+% endif
+% endfor
+
 for &(main_command_name, ref subcommands) in arg_data.iter() {
     let mut mcmd = SubCommand::new(main_command_name);
 
@@ -277,7 +283,7 @@ for &(main_command_name, ref subcommands) in arg_data.iter() {
                 arg = arg.multiple(multi);
             }
             if let &Some(ref protocols) = protocols {
-                arg = arg.possible_values(protocols.clone());
+                arg = arg.possible_values(protocols);
                 arg = arg.requires("file");
                 arg = arg.requires("mime");
 

src/mako/cli/lib/cli.py

@@ -25,6 +25,9 @@
 MIME_ARG = 'mime'
 OUT_ARG = 'out'
 
+SCOPE_ARG = 'url'
+CONFIG_DIR_ARG = 'folder'
+
 FIELD_SEP = '.'
 
 CONFIG_DIR = '~/.google-service-cli'
@@ -99,6 +102,9 @@ def req_value(name):
 def opt_value(name, opt='opt', default=''):
     return opt + '.value_of("' + mangle_subcommand(name) + ('").unwrap_or("%s")' % default)
 
+def opt_values(name, opt='opt'):
+    return opt + '.values_of("' + mangle_subcommand(name) + '").unwrap_or(Vec::new()).iter()'
+
 def application_secret_path(program_name):
     return program_name + '-secret.json'
 

src/mako/cli/lib/engine.mako

@@ -7,7 +7,8 @@
                      CONFIG_DIR, SCOPE_FLAG, is_request_value_property, FIELD_SEP, docopt_mode, FILE_ARG, MIME_ARG, OUT_ARG, 
                      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)
+                     application_secret_path, DEBUG_FLAG, DEBUG_AUTH_FLAG, CONFIG_DIR_FLAG, req_value, MODE_ARG, 
+                     opt_values, SCOPE_ARG, CONFIG_DIR_ARG)
 
     v_arg = '<%s>' % VALUE_ARG
     SOPT = 'self.opt'
@@ -28,7 +29,7 @@
     hub_type_name = 'api::' + hub_type(c.schemas, util.canonical_name())
 %>\
 use cmn::{InvalidOptionsError, CLIError, JsonTokenStorage, arg_from_str, writer_from_opts, parse_kv_arg, 
-          input_file_from_opts, input_mime_from_opts, FieldCursor, FieldError};
+          input_file_from_opts, input_mime_from_opts, FieldCursor, FieldError, UploadProtocol};
 
 use std::default::Default;
 use std::str::FromStr;
@@ -91,7 +92,7 @@ impl<'n, 'a> Engine<'n, 'a> {
     // Please note that this call will fail if any part of the opt can't be handled
     fn new(opt: ArgMatches<'a, 'n>) -> Result<Engine<'a, 'n>, InvalidOptionsError> {
         let (config_dir, secret) = {
-            let config_dir = match cmn::assure_config_dir_exists(opt.value_of("${CONFIG_DIR_FLAG}").unwrap_or("${CONFIG_DIR}")) {
+            let config_dir = match cmn::assure_config_dir_exists(opt.value_of("${CONFIG_DIR_ARG}").unwrap_or("${CONFIG_DIR}")) {
                 Err(e) => return Err(InvalidOptionsError::single(e, 3)),
                 Ok(p) => p,
             };
@@ -174,7 +175,7 @@ ${self._request_value_impl(c, request_cli_schema, prop_name, request_prop_type)}
     % elif p.type != 'string':
     % if p.get('repeated', False): 
 let ${prop_name}: Vec<${prop_type} = Vec::new();
-for (arg_id, arg) in opt.values_of("${mangle_subcommand(p.name)}").unwrap_or(Vec::new()).iter().enumerate() {
+for (arg_id, arg) in ${opt_values(mangle_subcommand(p.name))}.enumerate() {
     ${prop_name}.push(arg_from_str(&arg, err, "<${mangle_subcommand(p.name)}>", arg_id), "${p.type}"));
 }
     % else:
@@ -199,7 +200,7 @@ let mut download_mode = false;
 % endif
 let mut call = self.hub.${mangle_ident(resource)}().${mangle_ident(method)}(${', '.join(call_args)});
 % if handle_props:
-for parg in opt.values_of("${VALUE_ARG}").unwrap_or(Vec::new()).iter() {
+for parg in ${opt_values(VALUE_ARG)} {
     let (key, value) = parse_kv_arg(&*parg, err, false);
     match key {
 % for p in optional_props:
@@ -266,25 +267,23 @@ if dry_run {
 } else {
     assert!(err.issues.len() == 0);
     % if method_default_scope(mc.m):
-<% scope_opt = opt_value('scope', SOPT) %>\
-    if ${scope_opt}.len() > 0 {
-        call = call.${ADD_SCOPE_FN}(&${scope_opt});
+    for scope in ${opt_values(SCOPE_ARG, opt=SOPT)} {
+        call = call.${ADD_SCOPE_FN}(scope);
     }
     % endif
     ## Make the call, handle uploads, handle downloads (also media downloads|json decoding)
     ## TODO: unify error handling
     % if handle_output:
     let mut ostream = writer_from_opts(opt.value_of("${(OUT_ARG)}"));
     % endif # handle output
-    match match protocol {
+    match match UploadProtocol::from(protocol) {
         % if mc.media_params:
         % for p in mc.media_params:
-        "${p.protocol}" => call.${upload_action_fn(api.terms.upload_action, p.type.suffix)}(input_file.unwrap(), mime_type.unwrap()),
+        UploadProtocol::${p.protocol.capitalize()} => call.${upload_action_fn(api.terms.upload_action, p.type.suffix)}(input_file.unwrap(), mime_type.unwrap()),
         % endfor
         % else:
         "${STANDARD}" => call.${api.terms.action}(),
         % endif
-        _ => unreachable!(),
     } {
         Err(api_err) => Some(api_err),
         % if mc.response_schema:
@@ -366,7 +365,7 @@ if dry_run {
 %>\
 let mut ${request_prop_name} = api::${request_prop_type}::default();
 let mut field_cursor = FieldCursor::default();
-for kvarg in opt.values_of("${KEY_VALUE_ARG}").unwrap_or(Vec::new()).iter() {
+for kvarg in ${opt_values(KEY_VALUE_ARG)} {
     let last_errc = err.issues.len();
     let (key, value) = parse_kv_arg(&*kvarg, err, false);
     let mut temp_cursor = field_cursor.clone();

src/rust/cli/cmn.rs

@@ -16,8 +16,6 @@ use std::default::Default;
 
 const FIELD_SEP: char = '.';
 
-/// Unused for now, can be used once https://github.com/kbknapp/clap-rs/issues/87
-/// is available
 pub enum UploadProtocol {
     Simple,
     Resumable,
@@ -32,6 +30,16 @@ impl AsRef<str> for UploadProtocol {
     }
 }
 
+impl<'a> From<&'a str> for UploadProtocol {
+    fn from(this: &'a str) -> UploadProtocol {
+        match this {
+            "simple" => UploadProtocol::Simple,
+            "resumable" => UploadProtocol::Resumable,
+            _ => unreachable!(),
+        }
+    }
+}
+
 #[derive(Clone, Default)]
 pub struct FieldCursor(Vec<String>);