[tarantool-patches] Re: [PATCH v2 4/4] app: allow to raise an error on too nested tables

[Alexander Turenko]

>     YAML is a crazy serializer without depth restrictions. But for
>     JSON and msgpack a user could set encode_max_depth option. That

Nit: And msgpackffi.

>     Option encode_crop_too_deep works for JSON and msgpack modules,

Nit: And msgpackffi.

> >>     msgpack.cfg({encode_crop_too_deep = false})
> > 
> > We have encode_invalid_as_nil with the similar meanings, so, maybe,
> > encode_too_deep_as_nil?
> > 
> > Or maybe rephrase 'too deep' as one word: foots, dregs, underside? Those
> > variants don't look good, but maybe you know some word that would fit
> > better.
> 
> No, I don't know a good synonym. But perhaps we could use
> 'encode_max_depth_and_crop'. That 1) makes clear, that the option is
> related to 'encode_max_depth', 2) is obvious, that data deeper than
> max_depth is cropped. Is it ok?

Words in 'encode_max_depth_and_crop' feels as non-concordant, IMHO.

> Or 'encode_crop_after_max_depth'. Or 'encode_trim_by_max_depth'.

For me 'encode_crop_after_max_depth' is better then
'encode_crop_too_deep'.

I stick a bit with the idea to use 'as_nil', but I dislike the word
'too' in an option name: it feels vague. Maybe 'encode_deep_as_nil'?

If we'll go away from 'as_nil', then 'encode_raise_after_max_depth' look
good (except maybe length).

>From all those variants I would choose 'encode_deep_as_nil'.

I would let you decide finally.

> > BTW, the default is 'false' and we should set the default value here
> > (now we set 'true'). Following test cases (e.g. gh-2888 test case in
> > app-tap/json.test.lua) should set and restore the option locally.
> > 
> > Maybe it also worth add a test case that will verify that the default
> > behaviour is to raise an error?
> > 
> 
> Not sure. Such a test would assume a certain value of
> msgpack.cfg.encode_crop_too_deep. This is exactly what I was trying to
> avoid in the tests.

Okay.