-
Notifications
You must be signed in to change notification settings - Fork 117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make it possible to ask for a *[]T #16
Labels
enhancement
New feature or request
help wanted
Issues that anyone could pick up and implement if useful to them
needs use cases
Feature we can add if people find use cases that need it (comment if that's you)
Comments
benjaminjkraft
added a commit
that referenced
this issue
Aug 20, 2021
In this commit I remove one of the limitations of our support for interfaces, from #52, by adding support for list-of-interface fields. This was surprisingly complex! The issue is that, as before, it's the containing type that has to do all the glue work -- and it's that glue work that is complicated by list-of-interface fields. All in all, it's not that much new code, and by far the hard part is just 20 lines in the UnmarshalJSON template (which come with almost twice as many lines of comments to explain them). It may be easiest to start by reading some of the generated code, and then read the template. I also added support for such fields with `pointer: true` specified, such that the type is `[][]...[]*MyInterface`, although I don't know why you would want that. This does *not* allow e.g. `*[]*[][]*MyInterface`; that would require a way to specify it (see #16) but also add some extra complexity (as we'd have to actually walk the type-unwrap chain properly, instead of just counting the number of slices and whether there's a pointer). Issue: #8 Test plan: make check Reviewers: marksandstrom, miguel, csilvers, adam
benjaminjkraft
added a commit
that referenced
this issue
Aug 20, 2021
In this commit I remove one of the limitations of our support for interfaces, from #52, by adding support for list-of-interface fields. This was surprisingly complex! The issue is that, as before, it's the containing type that has to do all the glue work -- and it's that glue work that is complicated by list-of-interface fields. All in all, it's not that much new code, and by far the hard part is just 20 lines in the UnmarshalJSON template (which come with almost twice as many lines of comments to explain them). It may be easiest to start by reading some of the generated code, and then read the template. I also added support for such fields with `pointer: true` specified, such that the type is `[][]...[]*MyInterface`, although I don't know why you would want that. This does *not* allow e.g. `*[]*[][]*MyInterface`; that would require a way to specify it (see #16) but also add some extra complexity (as we'd have to actually walk the type-unwrap chain properly, instead of just counting the number of slices and whether there's a pointer). Issue: #8 Test plan: make check Reviewers: marksandstrom, miguel, csilvers, adam
benjaminjkraft
added a commit
that referenced
this issue
Aug 20, 2021
In this commit I remove one of the limitations of our support for interfaces, from #52, by adding support for list-of-interface fields. This was surprisingly complex! The issue is that, as before, it's the containing type that has to do all the glue work -- and it's that glue work that is complicated by list-of-interface fields. All in all, it's not that much new code, and by far the hard part is just 20 lines in the UnmarshalJSON template (which come with almost twice as many lines of comments to explain them). It may be easiest to start by reading some of the generated code, and then read the template. I also added support for such fields with `pointer: true` specified, such that the type is `[][]...[]*MyInterface`, although I don't know why you would want that. This does *not* allow e.g. `*[]*[][]*MyInterface`; that would require a way to specify it (see #16) but also add some extra complexity (as we'd have to actually walk the type-unwrap chain properly, instead of just counting the number of slices and whether there's a pointer). Issue: #8 Test plan: make check Reviewers: marksandstrom, miguel, csilvers, adam
benjaminjkraft
added a commit
that referenced
this issue
Aug 23, 2021
In this commit I remove one of the limitations of our support for interfaces, from #52, by adding support for list-of-interface fields. This was surprisingly complex! The issue is that, as before, it's the containing type that has to do all the glue work -- and it's that glue work that is complicated by list-of-interface fields. All in all, it's not that much new code, and by far the hard part is just 20 lines in the UnmarshalJSON template (which come with almost twice as many lines of comments to explain them). It may be easiest to start by reading some of the generated code, and then read the template. I also added support for such fields with `pointer: true` specified, such that the type is `[][]...[]*MyInterface`, although I don't know why you would want that. This does *not* allow e.g. `*[]*[][]*MyInterface`; that would require a way to specify it (see #16) but also add some extra complexity (as we'd have to actually walk the type-unwrap chain properly, instead of just counting the number of slices and whether there's a pointer). Issue: #8 Test plan: make check Reviewers: marksandstrom, miguel, csilvers, adam
benjaminjkraft
added a commit
that referenced
this issue
Aug 23, 2021
In this commit I remove one of the limitations of our support for interfaces, from #52, by adding support for list-of-interface fields. This was surprisingly complex! The issue is that, as before, it's the containing type that has to do all the glue work -- and it's that glue work that is complicated by list-of-interface fields. All in all, it's not that much new code, and by far the hard part is just 20 lines in the UnmarshalJSON template (which come with almost twice as many lines of comments to explain them). It may be easiest to start by reading some of the generated code, and then read the template. I also added support for such fields with `pointer: true` specified, such that the type is `[][]...[]*MyInterface`, although I don't know why you would want that. This does *not* allow e.g. `*[]*[][]*MyInterface`; that would require a way to specify it (see #16) but also add some extra complexity (as we'd have to actually walk the type-unwrap chain properly, instead of just counting the number of slices and whether there's a pointer). Issue: #8 Test plan: make check Reviewers: marksandstrom, miguel, csilvers, adam
benjaminjkraft
added a commit
that referenced
this issue
Aug 25, 2021
In this commit I remove one of the limitations of our support for interfaces, from #52, by adding support for list-of-interface fields. This was surprisingly complex! The issue is that, as before, it's the containing type that has to do all the glue work -- and it's that glue work that is complicated by list-of-interface fields. All in all, it's not that much new code, and by far the hard part is just 20 lines in the UnmarshalJSON template (which come with almost twice as many lines of comments to explain them). It may be easiest to start by reading some of the generated code, and then read the template. I also added support for such fields with `pointer: true` specified, such that the type is `[][]...[]*MyInterface`, although I don't know why you would want that. This does *not* allow e.g. `*[]*[][]*MyInterface`; that would require a way to specify it (see #16) but also add some extra complexity (as we'd have to actually walk the type-unwrap chain properly, instead of just counting the number of slices and whether there's a pointer). Issue: #8 Test plan: make check Reviewers: marksandstrom, miguel, csilvers, adam
benjaminjkraft
added a commit
that referenced
this issue
Aug 25, 2021
## Summary: In this commit I remove one of the limitations of our support for interfaces, from #52, by adding support for list-of-interface fields. This was surprisingly complex! The issue is that, as before, it's the containing type that has to do all the glue work -- and it's that glue work that is complicated by list-of-interface fields. All in all, it's not that much new code, and by far the hard part is just 20 lines in the UnmarshalJSON template (which come with almost twice as many lines of comments to explain them). It may be easiest to start by reading some of the generated code, and then read the template. I also added support for such fields with `pointer: true` specified, such that the type is `[][]...[]*MyInterface`, although I don't know why you would want that. This does *not* allow e.g. `*[]*[][]*MyInterface`; that would require a way to specify it (see #16) but also add some extra complexity (as we'd have to actually walk the type-unwrap chain properly, instead of just counting the number of slices and whether there's a pointer). Issue: #8 ## Test plan: make check Author: benjaminjkraft Reviewers: dnerdy, benjaminjkraft, aberkan, csilvers, MiguelCastillo Required Reviewers: Approved by: dnerdy Checks: ⌛ Test (1.17), ⌛ Test (1.16), ⌛ Test (1.15), ⌛ Test (1.14), ⌛ Test (1.13), ⌛ Lint, ⌛ Lint, ⌛ Test (1.17), ⌛ Test (1.16), ⌛ Test (1.15), ⌛ Test (1.14), ⌛ Test (1.13) Pull request URL: #54
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
enhancement
New feature or request
help wanted
Issues that anyone could pick up and implement if useful to them
needs use cases
Feature we can add if people find use cases that need it (comment if that's you)
If you put
pointer: true
on a list-field, you get[]*T
(or[][]*T
, or...) which is probably what you wanted (so you can represent an required list of optional items, for example, but not the only option. If anyone finds a reason to want*[]T
, or for that matter*[][]*[]T
, we can figure out a syntax for it; it's easy enough to do technically. (Although it won't be totally trivial ifT
is an interface.)The text was updated successfully, but these errors were encountered: