Code review comment for lp:~paul-lucas/zorba/feature-json_strip_array

Revision history for this message
Ghislain Fourny (gislenius) wrote :

Hi Paul,

Thanks for doing this. I like the parsing code.

1. Would it be possible to add tests to cover the new jsoniq-strip-top-level-array flag?

2. I am wondering if the behaviour with both flags set should not be to strip all top-level arrays, not just the first one. For example:

jn:parse-json(
  '[ 1, 2, 3 ][ 4, 5, 6 ]',
  {
    "jsoniq-multiple-top-level-items" : true,
    "jsoniq-strip-top-level-array" : true
  })

returns 1 2 3[4, 5, 6]

but I would expect 1 2 3 4 5 6.

3. I am under the impression that we use different terminologies in Zorba to describe the same behaviour ('skip', 'strip', I also saw 'chop' in a discussion). It would be nice if we could unify, not sure if still possible though.

I hope it helps!

review: Needs Fixing

« Back to merge proposal