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

Added valueForKey filter #191

Closed
wants to merge 1 commit into from
Closed

Added valueForKey filter #191

wants to merge 1 commit into from

Conversation

ilyapuchka
Copy link
Collaborator

This PR implements the same functionality as #174 but with less confusing API.

@ilyapuchka ilyapuchka requested a review from kylef January 22, 2018 16:11
Copy link
Collaborator

@kylef kylef left a comment

Choose a reason for hiding this comment

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

Started reviewing, but then thought about #189 (comment), submitting comment review for posterity. But perhaps these two filters can be combined.


func valueForKeyFilter(value: Any?, arguments: [Any?], context: Context) throws -> Any? {
guard arguments.count == 1 else {
throw TemplateSyntaxError("'split' filter takes a single argument")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like copy/paste error with filter name (split -> valueForKey)

let result = try template.render(Context(dictionary: ["key": "name", "value": ["name": "One"]]))
try expect(result) == "One"
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it might be a good idea to include a more complex example which validates that the filter uses the logic for variable lookup (perhaps in future some refactoring could break this currently supported behaviour):

let context: [String: Any] = [
    "user": ["name": "Kyle"]
]
let template = Template(templateString: "{{ value|valueForKey:"user.name" }}")


let key = stringify(arguments[0])
return try context.push(dictionary: ["filter_value": value as Any]) {
return try Variable("filter_value.\(key)").resolve(context)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it would make sense that this could handle arrays, similar to NSArray valueForKey:.

This would allow chaining valueForKey with join.

{{ users|valueForKey:"name"|join }}

@kylef kylef added this to the 0.11.0 milestone Jan 22, 2018
@ilyapuchka
Copy link
Collaborator Author

@kylef I've added map for single value in #189 so closing this

@ilyapuchka ilyapuchka closed this Jan 28, 2018
@ilyapuchka ilyapuchka deleted the value-for-key-filter branch January 28, 2018 10:23
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.

2 participants