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

config: Audit all interpolation functions for list/map behavior #7834

Merged
merged 1 commit into from
Jul 28, 2016

Conversation

phinze
Copy link
Contributor

@phinze phinze commented Jul 27, 2016

  • distinct() - error on non-flat lists
  • element() - error on non-flat lists
  • join() - error on non-flat lists
  • length() - support maps
  • lookup() - error on non-flat maps
  • values() - error on non-flat maps

@phinze
Copy link
Contributor Author

phinze commented Jul 27, 2016

cc @jen20 @jbardin

@@ -487,11 +491,16 @@ func interpolationFuncDistinct() ast.Function {
var list []string

if len(args) != 1 {
return nil, fmt.Errorf("distinct() excepts only one argument.")
return nil, fmt.Errorf("accepts only one argument.")
Copy link
Contributor

Choose a reason for hiding this comment

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

distinct() accepts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I learned that the caller prepends the function name automatically, so we were getting stutter in the resulting err messages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Confirmed error prints like this:

Errors:

  * distinct: only works for flat lists, this list contains elements of type list in:

${distinct(list(list(","), list(",")))}

@jen20
Copy link
Contributor

jen20 commented Jul 27, 2016

Minor nitpicks about error message consistency and a couple of typos in the docs, but otherwise LGTM!

 - `distinct()` - error on non-flat lists
 - `element()` - error on non-flat lists
 - `join()` - error on non-flat lists
 - `length()` - support maps
 - `lookup()` - error on non-flat maps
 - `values()` - error on non-flat maps
@phinze phinze force-pushed the f-interp-funcs-list-map-audit branch from 3ca1de0 to 8803076 Compare July 28, 2016 14:58
@phinze
Copy link
Contributor Author

phinze commented Jul 28, 2016

Okay addressed all the comments - will merge on green.

@phinze phinze merged commit 3f16a06 into master Jul 28, 2016
@phinze phinze deleted the f-interp-funcs-list-map-audit branch July 28, 2016 15:20
@ghost
Copy link

ghost commented Apr 23, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants