ppx: allow dropping arbitrary values with [@drop_default]#77
ppx: allow dropping arbitrary values with [@drop_default]#77zazedd wants to merge 1 commit intomelange-community:mainfrom
[@drop_default]#77Conversation
|
I wonder if we can do the following:
that way we won't be relying on structural equality |
|
Sounds good. To be clear, we want to replace structural equality by
This should be easy to add, but I'll just leave it commented out since |
Yes
Can keep that too, I guess.
Let's add it? Should be pretty easy. |
994e673 to
4b1cc93
Compare
|
Updated
Please check correctness of |
andreypopp
left a comment
There was a problem hiding this comment.
thanks! (left a small nit comment though)
| | Ptyp_constr ({ txt = Lident name; _ }, args) -> | ||
| let fn = pexp_ident ~loc { loc; txt = lident ("equal_" ^ name) } in | ||
| List.fold_left (List.rev args) ~init:fn ~f:(fun acc arg -> | ||
| pexp_apply ~loc acc [ Nolabel, equal_of_core_type ~loc arg ]) | ||
| | Ptyp_constr ({ txt = Ldot (prefix, name); _ }, args) -> | ||
| let name = if name = "t" then "equal" else "equal_" ^ name in | ||
| let fn = pexp_ident ~loc { loc; txt = Ldot (prefix, name) } in | ||
| List.fold_left (List.rev args) ~init:fn ~f:(fun acc arg -> | ||
| pexp_apply ~loc acc [ Nolabel, equal_of_core_type ~loc arg ]) |
There was a problem hiding this comment.
There was a problem hiding this comment.
Oh nice, I didn't know about this. Updated
|
@anmonteiro @jchavarri @davesnx anything we missed here? If no objections/comments, I'm going to merge in the coming days. |
|
I might prefer to have but that can also be added/improved later |
This PR allows
[@drop_default]to drop default values provided by[@default].It also changes
[@drop_default]to potentially accept a function for comparison. If no function is provided, then structural equality is used.[@drop_default]with[@option]remains unchangedNot sure if
ppx_compareworks with melange. If yes I can change this PR to be closer to how ppx_yojson_conv implements their similar featureMotivation
It brings
melange-jsoncloser to being able to substitute atd, and makes sense as an extension of the[@drop_default]attribute, which currently makesmelange-jsonunable to fully express some records without making fields needlessly optional