Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jan 20, 2015

Conversation

seancribbs
Copy link

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.

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.
@seancribbs
Copy link
Author

This is needed for RIAK-1425

?assertEqual(ConvertedArgs, [{sample_size, 10}]),

InvalidTypeArg = [{"sample_size", "A"}],
?assertMatch({error, {conversion, _}}, validate({Spec, InvalidTypeArg, [], []})).
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is failing for me. The actual error returned by cuttlefish appears to be {error,"\"A\" can't be converted to an integer"}, which doesn't match the {error, {conversion, _}} pattern.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure your cuttlefish is up to date.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, good call. That fixed it for me.

@nickelization
Copy link
Contributor

👍 55bd9f8

borshop added a commit that referenced this pull request Jan 20, 2015
Introduce spec record for args and flags, datatypes, and validators.

Reviewed-by: nickelization
@seancribbs
Copy link
Author

@borshop merge

@borshop borshop merged commit 55bd9f8 into develop Jan 20, 2015
@seancribbs seancribbs deleted the feature/cuttlefish-datatypes branch January 21, 2015 15:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants