show: handle [@printer] in polymorphic variants#267
Conversation
|
Code generated by openapi-generator-cli for ocaml uses custom printers for polymorphic variant branches as a way of supporting jsonschema enum types in generated client library code. I would have expected (and the openapi-generator-cli authors did, too) that the annotations would work the same way as for regular variants, but instead, the custom printers are silently ignored. The fix in this PR would be very helpful. |
gasche
left a comment
gasche
left a comment
There was a problem hiding this comment.
Thanks for the ping! I reviewed the PR. There is a comment on code style, which is optional, but the lack of wrap_printer is probably a bug.
@ghuysmans apologies for the delay in reviewing this. Would you by chance be able to tell if you want to take care of your PR despite the long wait, or if you have moved to other things and would rather have someone else adopt it?
| | Some printer, Rtag(label, true (*empty*), []) -> | ||
| let label = label.txt in | ||
| Exp.case (Pat.variant label None) | ||
| [%expr [%e printer] fmt ()] |
There was a problem hiding this comment.
I think it would be easier to factorize the two cases better, the code is identical except for the second argument of Exp.case that depends on the optional printer attribute.
| | Some printer, Rtag(label, true (*empty*), []) -> | ||
| let label = label.txt in | ||
| Exp.case (Pat.variant label None) | ||
| [%expr [%e printer] fmt ()] |
There was a problem hiding this comment.
I am worried about the use of printer directly when all other such patterns in the code use wrap_printer quoter printer. I don't remember the details of how this works, but I would follow the rest of the code here.
| | Some printer, Rtag(label, false, [typ]) -> | ||
| let label = label.txt in | ||
| Exp.case (Pat.variant label (Some [%pat? x])) | ||
| [%expr [%e printer] fmt x] |
There was a problem hiding this comment.
Same remarks: you could share more code (but not doing so is okay), and printer should be wrapped.
3 participants
No description provided.