customization of default rendering #414
Conversation
| ```kotlin | ||
| use smithy4s.meta#defaultRender | ||
|
|
||
| @defaultRender(mode: "FULL") |
There was a problem hiding this comment.
not quite sure it should be a trait. The preference probably applies to all case classes that users generate, so I'm thinking it should be a piece of metadata instead, that'd apply globally.
Now the problem is that the rendering of trait values relies on the ordering of parameters. We might want to change it to named parameters instead, which would remove the need to know which ordering was applied during the rendering of upstream namespaces.
There was a problem hiding this comment.
two things:
-
preference probably applies to all case classes
The opposite could be just as true I guess, some people may want to change the behaviour for one specific case class.
-
the rendering of trait values relies on the ordering of parameters
I don't understand what you mean by the rendering of trait values in this context, do you have an example?
There was a problem hiding this comment.
- we have optimise user-facing features in terms of what most users will benefit from. Though you are not technically incorrect, I'm pretty sure that if someone dislikes default values in one case, they'll dislike default values in all cases, because default values have very tangible downsides to them. Therefore, it's likely that a piece of UX that would allow to disable defaults altogether across the whole code-gen pipeline, people who dislike defaults will be happy about the low-boilerplate solution. They will be less happy about having to annotate all of their shapes.
- I meant rendering of hints.
There was a problem hiding this comment.
Good points, I forgot that we had talked about using the metadata for this. I'll update the PR with that and using named parameters in hints 👍
There was a problem hiding this comment.
Done in: 0b9d512
lmk if this doesn't match what you are talking about wrt the named parameters in hints
There was a problem hiding this comment.
lmk if this doesn't match what you are talking about wrt the named parameters in hints
It is indeed
| `FULL` means that default values are rendered for all field types. For example: | ||
|
|
||
| ```kotlin | ||
| metadata defaultRenderMode = "FULL" |
There was a problem hiding this comment.
All the metadata bits that smithy4s surfaces/uses should be prefixed with smithy4s one way or another. If . are allowed in metadata keys, we could make it smithy4s.defaultRenderMode
There was a problem hiding this comment.
Unfortunately, dots aren't allowed. But I changed it to smithy4sDefaultRenderMode
| @@ -11,3 +13,10 @@ structure DefaultTest { | |||
| two: String = "test" | |||
| three: StringList = [] | |||
| } | |||
|
|
|||
| structure DefaultOrderingTest { | |||
There was a problem hiding this comment.
Mmm, I think it might be a good opportunity to add unit tests in the codegen module, since relying on our usual smoke-test way of doing things ain't gonna work (as metadata is meant to be unique across a smithy model).
Would you mind adding such a unit test, a little bit like the smithy-translate ones, where we dynamically parse a model and run it through the codegen pipeline, and assert that the output code is what we think it should be ?
There was a problem hiding this comment.
Yeah that's a good idea, will add 👍
Allows rendering defaults in three different modes: FULL, OPTION_ONLY, NONE.
See docs in PR for full explanation.