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

Add type parameters to generated lenses of generic structs. #1591

Merged
merged 1 commit into from
Feb 15, 2021

Conversation

rjwittams
Copy link
Collaborator

This gives type inference something to grab hold of when chaining
LensExt methods or using WidgetExt::lens.
It removes the need for providing annoying and hard to work out type arguments to some method calls (removed one in Tabs).

This changes generated lenses from unit structs to field structs to hold PhantomDatas, so construction is more involved. It also adds generic parameters to these structs, so their types are changed.
A new method wasn't generated and used for the const fields in the targets impl, as lifetime parameters on const functions are unstable.

This gives type inference something to grab hold of when chaining
LensExt methods or using WidgetExt::lens.
@rjwittams rjwittams force-pushed the lens-generic-phantom branch from e4961cd to ae6bdf8 Compare February 15, 2021 17:33
@rjwittams
Copy link
Collaborator Author

I realised I could use tuple structs to avoid making up idents for PhantomDatas, and I am skipping lifetime parameters for these anyway as I don't think they are needed - so have added and used the const fn new .

It would be possible to make the generated structs identical to their current form if they have no args.
I doubt that anyone is relying on this, and it would make this code a bit more complex (a few more conditionals etc).

Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be possible to make the generated structs identical to their current form if they have no args.
I doubt that anyone is relying on this, and it would make this code a bit more complex (a few more conditionals etc).

Not sure I totally understand, do you mean possible to keep them unit structs?

iirc I have maybe once needed to go find the actual type of a generated lens, but I don't think we should expect anyone to ever use these directly; I think if they need to it's because we've done something wrong.

In any case I think this looks good and definitely improves ergonomics.

@rjwittams
Copy link
Collaborator Author

Yeah the only case it would matter is if someone is referencing the type directly as a value. Which seems unlikely, as we provide the const fields for that purpose.

@rjwittams rjwittams merged commit d846cc2 into linebender:master Feb 15, 2021
richard-uk1 pushed a commit to richard-uk1/druid that referenced this pull request Apr 6, 2021
…er#1591)

This gives type inference something to grab hold of when chaining
LensExt methods or using WidgetExt::lens.
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.

2 participants