Skip to content
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

feat(client): provide a way to select interfaces fields #1169

Merged
merged 1 commit into from
Nov 25, 2021

Conversation

AlixBa
Copy link
Contributor

@AlixBa AlixBa commented Nov 24, 2021

Closes #237

@AlixBa AlixBa marked this pull request as ready for review November 24, 2021 15:59
@AlixBa AlixBa changed the title Draft: provide a way to select interfaces fields feat(client): provide a way to select interfaces fields Nov 24, 2021
@AlixBa
Copy link
Contributor Author

AlixBa commented Nov 24, 2021

@ghostdogpr is the 2nd commit close enough to what you had in mind for #237?

@ghostdogpr
Copy link
Owner

@AlixBa what I was thinking is a little different. Let's say that for each subtype you want to get the common fields from the interface, plus an extra field for one of the subtypes. To do that, you have to repeat the interface fields for each subtypes. So I was thinking about adding an extra parameter to the option function, that takes an optional SelectionBuilder of the interface. That way, for my example, you only need to pass this one and the SelectionBuilder for the subtype that needs an extra field. Does that make sense?

By the way I merged your other PR, so you can rebase to get rid of the conflicts.

@ghostdogpr
Copy link
Owner

ghostdogpr commented Nov 25, 2021

Ah sorry, what I suggested doesn't work 😆

def parentOption[A](
  onParent: Option[SelectionBuilder[Parent, A] = None,
  onSubType1: Option[SelectionBuilder[SubType1, A]] = None,
  onSubType2: Option[SelectionBuilder[SubType2, A]] = None
)

There is no way to merge the 2 A from the interface and the subtype.

So I think what you did is good. In my example, we can combine parentInterface and parentOption with ~ to select the common fields and the subtype ones. It would be nice to include a test for that.

@AlixBa
Copy link
Contributor Author

AlixBa commented Nov 25, 2021

@ghostdogpr I've added one. Not sure it is the right place/what you were expecting though ^^

@ghostdogpr
Copy link
Owner

Yep, that's good! Thanks!

@ghostdogpr ghostdogpr merged commit 9efd71b into ghostdogpr:master Nov 25, 2021
Fluxx pushed a commit to Fluxx/caliban that referenced this pull request Jan 24, 2022
Fluxx pushed a commit to Fluxx/caliban that referenced this pull request Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Caliban client: better helper for interfaces
2 participants