Introduce spec record for args and flags, datatypes, and validators.

This uses cuttlefish's datatype conversion routines to automatically
parse kvargs and flags. This relieves the burden of conversion from the
user and resolves the issue of needing to customize error messages
emitted from typecast functions, because cuttlefish already provides
them. See #44.

In order to maintain the same functionality provided by typecast (for
example, clique_typecast:to_node/1), an additional 'validator' option is
now recognized. The validator will be called after type conversion.

While adding these features, the proplist options seemed to be
complicating too much of the code, so the internal representation of an
arg or flag spec was converted to a record with all the necessary
fields. Users of the code will not need to change passed arguments
because the record is only used internally for efficiency and
clarity. Additionally, the record enables us to avoid calling
list_to_atom/1 on user input in the parser, and instead use simple
matching and lists:keyfind instead. All atom creation is done when
registering the command, improving safety.

Author: Sean Cribbs

include/clique_specs.hrl

@@ -0,0 +1,13 @@
+%% This record represents the specification for a key-value argument
+%% or flag on the command line.
+-record(clique_spec,
+        {
+          key        :: atom(),
+          name       :: string(),
+          shortname  :: char() | undefined,
+          datatype   :: cuttlefish_datatypes:datatype() | undefined,
+          validator  :: fun((term()) -> ok | err()) | undefined,
+          typecast   :: fun((string()) -> err() | term()) | undefined
+        }).
+
+-type spec() :: #clique_spec{}.

src/clique_command.erl

@@ -18,6 +18,7 @@
 %%
 %% -------------------------------------------------------------------
 -module(clique_command).
+-include("clique_specs.hrl").
 
 -define(cmd_table, clique_commands).
 
@@ -40,7 +41,9 @@ init() ->
 
 %% @doc Register a cli command (i.e.: "riak-admin handoff status")
 -spec register([string()], list(), list(), fun()) -> true.
-register(Cmd, Keys, Flags, Fun) ->
+register(Cmd, Keys0, Flags0, Fun) ->
+    Keys = make_specs(Keys0),
+    Flags = make_specs(Flags0),
     ets:insert(?cmd_table, {Cmd, Keys, Flags, Fun}).
 
 -spec run(err()) -> err();
@@ -86,6 +89,11 @@ split_command(Cmd0) ->
                         clique_parser:is_not_flag(Str)
                     end, Cmd0).
 
+
+-spec make_specs([{atom(), proplist()}]) -> [spec()].
+make_specs(Specs) ->
+    [ clique_spec:make(Spec) || Spec <- Specs ].
+
 %% NB This is a bit sneaky. We normally only accept key/value args like
 %% "handoff.inbound=off" and flag-style arguments like "--node [email protected]" or "--all",
 %% but the builtin "show" and "describe" commands work a bit differently.

src/clique_config.erl

@@ -18,6 +18,7 @@
 %%
 %% -------------------------------------------------------------------
 -module(clique_config).
+-include("clique_specs.hrl").
 
 %% API
 -export([init/0,
@@ -43,7 +44,9 @@
 -type err() :: {error, term()}.
 -type status() :: clique_status:status().
 -type proplist() :: [{atom(), term()}].
--type flags() :: [{atom() | char(), term()}].
+-type flagspecs() :: [spec()].
+-type flags() :: proplist().
+-type args() :: clique_parser:args().
 
 -type envkey() :: {string(), {atom(), atom()}}.
 -type cuttlefish_flag_spec() :: {flag, atom(), atom()}.
@@ -238,15 +241,15 @@ run_callback({Args, Flags, Status}) ->
     Status.
 
 -spec get_config(err()) -> err();
-                ({proplist(), flags()}) ->
+                ({args(), flags()}) ->
                     {proplist(), proplist(), flags()} | err().
 get_config({error, _}=E) ->
     E;
 get_config({[], _Flags}) ->
     {error, set_no_args};
 get_config({Args, Flags}) ->
     [{schema, Schema}] = ets:lookup(?schema_table, schema),
-    Conf = [{cuttlefish_variable:tokenize(atom_to_list(K)), V} || {K, V} <- Args],
+    Conf = [{cuttlefish_variable:tokenize(K), V} || {K, V} <- Args],
     case cuttlefish_generator:minimal_map(Schema, Conf) of
         {error, _, Msg} ->
             {error, {invalid_config, Msg}};
@@ -326,18 +329,18 @@ set_remote_app_config(AppConfig) ->
             [clique_status:alert([clique_status:text(Alert)])]
     end.
 
--spec config_flags() -> flags().
+-spec config_flags() -> flagspecs().
 config_flags() ->
-    [{node, [{shortname, "n"},
-             {longname, "node"},
-             {typecast, fun clique_typecast:to_node/1},
-             {description,
-                 "The node to apply the operation on"}]},
-
-     {all, [{shortname, "a"},
-            {longname, "all"},
-            {description,
-                "Apply the operation to all nodes in the cluster"}]}].
+    [clique_spec:make({node, [{shortname, "n"},
+                              {longname, "node"},
+                              {typecast, fun clique_typecast:to_node/1},
+                              {description,
+                               "The node to apply the operation on"}]}),
+
+     clique_spec:make({all, [{shortname, "a"},
+                             {longname, "all"},
+                             {description,
+                              "Apply the operation to all nodes in the cluster"}]})].
 
 
 -spec get_valid_mappings([string()]) -> err() | [{string(), cuttlefish_mapping:mapping()}].

src/clique_error.erl

@@ -81,7 +81,11 @@ format(_Cmd, {error, {config_not_settable, Keys}}) ->
 format(_Cmd, {error, {nodedown, Node}}) ->
     status(io_lib:format("Target node is down: ~p~n", [Node]));
 format(_Cmd, {error, bad_node}) ->
-    status("Invalid node name").
+    status("Invalid node name");
+format(_Cmd, {error, {conversion, _}}=TypeError) ->
+    %% Type-conversion error originating in cuttlefish
+    status(cuttlefish_error:xlate(TypeError)).
+
 
 -spec status(string()) -> status().
 status(Str) ->