Skip to content

Merge all WindowId into a single type in the core crate#3902

Merged
kchibisov merged 1 commit intomasterfrom
madsmtm/there-can-be-only-one-window-id
Oct 8, 2024
Merged

Merge all WindowId into a single type in the core crate#3902
kchibisov merged 1 commit intomasterfrom
madsmtm/there-can-be-only-one-window-id

Conversation

@madsmtm
Copy link
Copy Markdown
Member

@madsmtm madsmtm commented Sep 8, 2024

WindowId is a window identifier, and as such doesn't store anything (unlike a handle). So we can safely make only be defined once, in the core crate.

There are a few backends where we still use into_raw internally; I consider these patterns discouraged, we should not be passing around important state in the window id.

Builds upon #3864. Part of #3433.

@madsmtm madsmtm force-pushed the madsmtm/there-can-be-only-one-window-id branch from eea3202 to 4be1aac Compare September 8, 2024 11:48
@madsmtm madsmtm added the S - api Design and usability label Sep 10, 2024
@kchibisov
Copy link
Copy Markdown
Member

Linked PR got merged, so this should be unblocked now.

@kchibisov kchibisov force-pushed the madsmtm/there-can-be-only-one-window-id branch from 4be1aac to 7e054cb Compare September 29, 2024 17:14
@kchibisov kchibisov marked this pull request as ready for review September 29, 2024 17:14
@kchibisov kchibisov force-pushed the madsmtm/there-can-be-only-one-window-id branch from 7e054cb to 3a8e5fc Compare September 29, 2024 17:16
Copy link
Copy Markdown
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

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

I've rebased, other than that I don't have any issues.

@kchibisov kchibisov force-pushed the madsmtm/there-can-be-only-one-window-id branch from 3a8e5fc to a53ba65 Compare September 29, 2024 17:47
Copy link
Copy Markdown
Member

@daxpedda daxpedda left a comment

Choose a reason for hiding this comment

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

Adjusted Web a bit.
LGTM!

@kchibisov kchibisov force-pushed the madsmtm/there-can-be-only-one-window-id branch from 80f5712 to 0697847 Compare October 8, 2024 13:10
@kchibisov kchibisov force-pushed the madsmtm/there-can-be-only-one-window-id branch from 0697847 to 0eb96de Compare October 8, 2024 13:19
WindowId is a window _identifier_, and as such doesn't store anything
(unlike a _handle_). So we can safely make only be defined once, in the
core crate.

There are a few backends where we still use `into_raw` internally; I
consider these patterns discouraged, we should not be passing around
important state in the window id.
@kchibisov kchibisov force-pushed the madsmtm/there-can-be-only-one-window-id branch from 0eb96de to 954befe Compare October 8, 2024 13:24
@kchibisov kchibisov merged commit da2268a into master Oct 8, 2024
@kchibisov kchibisov deleted the madsmtm/there-can-be-only-one-window-id branch October 8, 2024 13:29
@madsmtm madsmtm mentioned this pull request May 15, 2025
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S - api Design and usability

Development

Successfully merging this pull request may close these issues.

3 participants