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

Type-system relationship between HasRaw*Handle and Has*Handle traits #135

Closed
madsmtm opened this issue Jul 28, 2023 · 3 comments · Fixed by #139
Closed

Type-system relationship between HasRaw*Handle and Has*Handle traits #135

madsmtm opened this issue Jul 28, 2023 · 3 comments · Fixed by #139
Labels
enhancement New feature or request

Comments

@madsmtm
Copy link
Member

madsmtm commented Jul 28, 2023

Should there be a type-system relationship between these?

I think it would make sense to make the normal handles subtraits of the raw handles, i.e. trait HasWindowHandle: HasRawWindowHandle { ... }?

Or maybe we should do a blanket impl<T: HasWindowHandle> for HasRawWindowHandle {}, or vice-versa?

@madsmtm madsmtm added the enhancement New feature or request label Jul 28, 2023
@madsmtm madsmtm changed the title Type-system relationship between Raw*Handle and *Handle traits Type-system relationship between HasRaw*Handle and Has*Handle traits Jul 28, 2023
@madsmtm
Copy link
Member Author

madsmtm commented Jul 28, 2023

Related: What is the value even in having two traits? Is it just that way because we're yet unsure of the design?

I think it'd make sense to combine them into a single HasWindowHandle/HasDisplayHandle trait.

@notgull
Copy link
Member

notgull commented Jul 28, 2023

This change would make the semver trick hard to do... but #136 would already make it impossible so why not just go full ham?

I've often compared the difference between HasRawWindowHandle and HasWindowHandle to the difference between AsRawFd and AsFd in the standard library. However, I struggle to think of a use case where a type would implement HasRawWindowHandle and not HasWindowHandle. The standard library has to keep AsRawFd forever to avoid a breaking change, but we don't.

I would be in favor of just merging the traits.

@madsmtm
Copy link
Member Author

madsmtm commented Jul 28, 2023

I think the semver trick is basically already impossible, see #125 (comment).

notgull added a commit that referenced this issue Aug 8, 2023
After reading #135, I've realized that the HasRawDisplayHandle and
HasRawWindowHandle traits have little value in a post-borrowed-handle
world. Borrowed handles do everything better, and the raw handle can be
extracted from the borrowed handle. Therefore it makes sense to remove
these traits.

Closes #135.

Signed-off-by: John Nunley <dev@notgull.net>
notgull added a commit that referenced this issue Sep 2, 2023
After reading #135, I've realized that the HasRawDisplayHandle and
HasRawWindowHandle traits have little value in a post-borrowed-handle
world. Borrowed handles do everything better, and the raw handle can be
extracted from the borrowed handle. Therefore it makes sense to remove
these traits.

Closes #135.

Signed-off-by: John Nunley <dev@notgull.net>
notgull added a commit that referenced this issue Sep 10, 2023
After reading #135, I've realized that the HasRawDisplayHandle and
HasRawWindowHandle traits have little value in a post-borrowed-handle
world. Borrowed handles do everything better, and the raw handle can be
extracted from the borrowed handle. Therefore it makes sense to remove
these traits.

Closes #135.

Signed-off-by: John Nunley <dev@notgull.net>
notgull added a commit that referenced this issue Sep 10, 2023
After reading #135, I've realized that the HasRawDisplayHandle and
HasRawWindowHandle traits have little value in a post-borrowed-handle
world. Borrowed handles do everything better, and the raw handle can be
extracted from the borrowed handle. Therefore it makes sense to remove
these traits.

Thus far I've deprecated them instead of removing them, in order to be more kind to downstream crates.

Closes #135.

Signed-off-by: John Nunley <dev@notgull.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging a pull request may close this issue.

2 participants