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 an separator parameter to Joi.reach #1383

Closed
pierreis opened this issue Dec 18, 2017 · 8 comments
Closed

Add an separator parameter to Joi.reach #1383

pierreis opened this issue Dec 18, 2017 · 8 comments
Assignees
Labels
feature New functionality or improvement
Milestone

Comments

@pierreis
Copy link

Currently, the separator of Joi.reach is fixed as a dot. This might not be a good choices for for use cases where object keys are e-mails or URLs, causing the method not to behave as expected.

I would suggest adding a third parameter separator to that method, allowing to be user-configured. For example, slashes can make sense for compatibility with JSON patch. The default would remain a dot for compatibility.

@DavidTPate
Copy link
Contributor

@pierreis We're always glad to take a look at PRs and do our best to get them merged, if this is something you'd like to see I'd suggesting giving it a stab in a PR.

I would suggest utilizing an options object as the third parameter as there might be some other options that are desired in the future and I would rather see an object for options as opposed more and more parameters.

As an example, Hoek.reach() supports a number of parameters and you can see the relevant code here.

@Marsup
Copy link
Collaborator

Marsup commented Dec 22, 2017

I'd rather take an array directly instead of adding new options.

@pierreis pierreis changed the title Add an separameter parameter to Joi.reach Add an separator parameter to Joi.reach Dec 22, 2017
@pierreis
Copy link
Author

That would solve the issue, indeed.
Considering in that case the current defaults (splitting with a dot separator) for backward compatibility.

@DavidTPate
Copy link
Contributor

Makes sense to me.

@mrveera
Copy link

mrveera commented Jan 17, 2018

@Marsup, I am ready to solve this issue by adding separator parameter to reach function

@Marsup
Copy link
Collaborator

Marsup commented Feb 3, 2018

I wrote a few posts above that I don't want this parameter, but you're free to do a PR following those instructions. Please ask if it's not clear.

@vijaykrishnavanshi
Copy link

Hi @Marsup, just to make sure that I am understanding the issue.
You mean array in the path param right?

@Marsup
Copy link
Collaborator

Marsup commented Feb 12, 2018

Correct, a pre-split array instead of the current path.

@Marsup Marsup added this to the 13.2.0 milestone Mar 9, 2018
@Marsup Marsup self-assigned this Mar 9, 2018
@Marsup Marsup closed this as completed Mar 9, 2018
@hueniverse hueniverse added feature New functionality or improvement and removed request labels Sep 19, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Mar 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New functionality or improvement
Projects
None yet
Development

No branches or pull requests

6 participants