Skip to content

Depth parameter #3

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Depth parameter #3

wants to merge 2 commits into from

Conversation

mastef
Copy link

@mastef mastef commented Jan 19, 2016

We needed an option to require a maximum depth of nesting. The default behaviour still works with specifying a negative value -1, no nesting is done with 0 and depth can be specified with any values above.

I don't know if you still maintain the project, but if this is interesting for you please feel free to merge, then I'd remove our fork. Otherwise feel free to ignore :-)

Added also some further test cases to test for the depth and a different merge character.

Note : This is a breaking change, since a parameter was added.

Cheers!

@durple
Copy link
Contributor

durple commented Jan 23, 2016

Thanks for the conribution. This is a difficult one since it is a breaking change.

Happy to accept with the following change. Add a fuction that takes the depth parameter and the existing function can stay the same. The existing function should just be a passthrough to the one with a depth parameter. We'd need these for both explodeMap and explodeList

e.g.

func explodeMap(l interface{}, parent string, delimiter string) (map[string]interface{}, error) {
    explodeMapWithDepth(i, parent, delimiter, -1)    
}

and

func explodeMapWithDepth(l interface{}, parent string, delimiter string, depth int) (map[string]interface{}, error) {
...
...
}

@durple
Copy link
Contributor

durple commented Jan 23, 2016

Also, if you could squash your commits before submitting the PR, I'd really appreciate it since this is a single change.

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