Bugfix/error annotate #1008
Bugfix/error annotate #1008
Conversation
Added option for the color coding in `error.annotate`. Hope I did it right.
This fixes the `error.annotate` footnote number bug. It was a simple brainfart, the author was probably thinking he needed to invert the index when that was not actually needed.
|
The tests fail because default arguments are not supported in node 4 and there's also a typo in your code. Also you'll be lacking some additional tests to validate your change, this should go inside this block (https://github.com/hapijs/joi/blob/master/test/errors.js#L366), you can find inspiration from other tests to do your owns. Let me know if you need more help. |
|
Alright, will try to fix these problems. I assume that since I can't use default arguments, that I should use null-checking? Also, a question: I noticed that while this makes the references correct, their order still does not feel quite natural. I'd like to also change it so that they come in the natural order from top to bottom. However, this does not seem that easy. What can I assume about the order of Error.details? And I see you're doing some specific stuff with the order (a comment talks about taking the innermost children first), so maybe there are some important things you're doing there? PS: I'm very impressed by your test coverage. |
Also changed the ordering a bit, but still not satisfied
|
@Marsup Would be nice to know how to do this ordering thing correctly. It seems that by making the ordering in objects kinda better, I ruined the ordering when the processed item is an array. Or something. |
|
I don't think the order matters much, maybe it would help you to have a look at the object before it's stringified as it contains all the placeholders that will be replaced, either that loop is wrong or the following one is. It would be a lot easier and clearer to land those 2 features separately though if you don't mind. |
Side-effect: Multiple references on one line will look slightly worse than before, but it's still correct, and at least the bugs are gone. Color-removal also works.
|
Allright, the color stuff works and the refernces are now correct - the order is still suboptimal (actually a bit uglier in one case) but yes, that can be fixed in a separate PR. |
|
To have the correct order you'd need a more complex processing, that's not the goal of annotate, as long as you can pinpoint the errors it's fine. I'll check and report back. |
|
It's now published, thanks a lot ! |
|
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. |
Fixes #1005 (and I guess #1006 ?)