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

add key-value parsing function #77

Merged
merged 4 commits into from Aug 8, 2016
Merged

add key-value parsing function #77

merged 4 commits into from Aug 8, 2016

Conversation

@kroepke
Copy link
Member

@kroepke kroepke commented Aug 4, 2016

fixes #38

@kroepke kroepke added this to the 1.1.0 milestone Aug 4, 2016

public KeyValue() {
valueParam = string("value").build();
splitParam = string("delimiter", CharMatcher.class).transform(CharMatcher::anyOf).optional().build();

This comment has been minimized.

@joschi

joschi Aug 4, 2016
Contributor

Since there is possibly more than 1 delimiter, the parameter should be called delimiters.

public KeyValue() {
valueParam = string("value").build();
splitParam = string("delimiter", CharMatcher.class).transform(CharMatcher::anyOf).optional().build();
valueSplitParam = string("kv_delimiter", CharMatcher.class).transform(CharMatcher::anyOf).optional().build();

This comment has been minimized.

@joschi

joschi Aug 4, 2016
Contributor

Since there is possibly more than 1 delimiter, the parameter should be called kv_delimiters.

This comment has been minimized.

@kroepke

kroepke Aug 5, 2016
Author Member

Good points!

@bernd
Copy link
Member

@bernd bernd commented Aug 8, 2016

@kroepke This also needs a rebase, thanks!

@kroepke kroepke force-pushed the kv-parse branch from 51a0787 to 117caaa Aug 8, 2016
@kroepke
Copy link
Member Author

@kroepke kroepke commented Aug 8, 2016

@bernd pushed

@Override
public Map<String, String> evaluate(FunctionArgs args, EvaluationContext context) {
final String value = valueParam.required(args, context);
if (value == null) {

This comment has been minimized.

@bernd

bernd Aug 8, 2016
Member

Does it make sense to short cut here for an empty value as well? (i.e. value="" or value=" ")

This comment has been minimized.

@kroepke

kroepke Aug 8, 2016
Author Member

Good point!

@bernd
Copy link
Member

@bernd bernd commented Aug 8, 2016

LGTM 👍

@bernd bernd merged commit 812b245 into master Aug 8, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@bernd bernd deleted the kv-parse branch Aug 8, 2016
edmundoa added a commit that referenced this pull request Aug 30, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants