@Marsup Thanks for your feedback. I've fixed what I could.

Can you explain what is the purpose of strip option? I couldn't find a good example in the lib/array.js.
I also couldn't find the rogue LF for test/array.js.

The strip flag is a consequence of the call of any.strip(), it's used just below, you should be able to understand the goal with the docs.
The LF is there in the github diff, so it's probably there in your source as well :)

@Marsup Thanks for your comment... Please review. Got the strip() implemented now. Missed the LF because JSlint wasn't complaining. Let me know if there are any more changes to do.

@Marsup Please review.

Sorry for the delay, busy days... One last thing, would you mind including all the w things into the loop parameters ? After that, if you know how to, can you rebase on current master and squash those commits ?

No worries.
Pretty scary to rebase and squash. Hope that works out alright.

Hey, sorry again I'm a bit slow on this one, I had to think about it for a while :)
There are 2 last problems that have been bothering me :

• it will crash if the number of items in the array is higher than the number of items
• I think we should be able to specify optional items, like Joi.array().items(a.required(), b.required(), c, d, e) where the array should be at least 2 items (a, b) and an infinite variation of c, d or e. Thoughts ?

Does it look like something you can achieve ?

No worries. I don't have a specific need for this feature. I'm contributing to learn. So, no rush...

• The first problem can be solved by better error handling, I think it also exists without ordered(). I can definitely help out with that.
• The use case for ordered() is to have strict validation on the array of items. Can the use case for optional items be met without the ordered() flag?

The 1st problem doesn't exist currently since loops on values and schemas are not tied together, unlike what you have to do here.

The use case cannot be covered with current API. I'm starting to think an even more powerful construct would be for ordered not to be a flag but to accept a list of schemas like items.

Can you explain more in detail because I'm a bit lost here?

Like Joi.array().ordered(Joi.string().valid('woohoo').required(), Joi.number().required(), Joi.string(), Joi.number()) where it would match ['woohoo', 2, 789, 'any string', 'from here']?

What about count of items in array? Should it pass if it doesn't match? Maybe not? Because that use case can be validated against length?

Let's take that example :

Joi.array()
  .ordered(Joi.string().required(), Joi.boolean().required(), Joi.number())
  .items(Joi.date())

It would match anything containing at least a string and a boolean, optionally a number, then any number of dates.
It seems like a useful addition to me.

+1 on the PR as it is. My main use case is supporting tuples (e.g. to serialize ordered maps as arrays of key/value tuples).

I'm not sure how common the use case @Marsup is describing is as I haven't come across it myself yet.

I think a more common example would be .ordered(...).items(joi.any()) -- where you only care about the types of the first N items in a mixed array and don't care if it contains anything else after them.

Nice...!! I will give that a shot.

Sorry, I didn't get to work on this PR this week. 😞 And it would need to be put on hold for a couple of weeks while I'm travelling.

I think we're missing a test case (which would probably fail) where there are fewer elements than the number of required ordered items.

Yes. You are right. That test case will fail. 😞 I will fix it tomorrow.

@Marsup Please review.

Very good job so far, we're getting to the final details :) 👍

Thanks, @Marsup. Please review.

Done. Please review.

Those were the final ones, ready to merge after that :)

Cool. All done now. :)

Congrats, hope it was not too painful :)

Nice!! It was more fun than painful. :) Do let me know if you have other
issues that you would like help with...
Thanks.

-CK-
On 5 Oct 2015 23:38, "Nicolas Morel" notifications@github.com wrote:

Congrats, hope it was not too painful :)

—
Reply to this email directly or view it on GitHub
#688 (comment).

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.