Skip to content

Remove #[non_exhaustive] on enum Ime - #4682

Merged
ogoffart merged 1 commit into
rust-windowing:masterfrom
dhardy:push-poxpnoppntnm
Sep 7, 2026
Merged

Remove #[non_exhaustive] on enum Ime#4682
ogoffart merged 1 commit into
rust-windowing:masterfrom
dhardy:push-poxpnoppntnm

Conversation

@dhardy

@dhardy dhardy commented Sep 6, 2026

Copy link
Copy Markdown
Contributor
  • Tested on all platforms changed
  • Added an entry to the changelog module if knowledge of this change could be valuable to users
  • Updated documentation to reflect any user-facing changes, including notes of platform-specific behavior
  • Created or updated an example program if it would help users understand this functionality

This is a partial revert of #4647.

Enum Ime should not be #[non_exhaustive] since ignoring variants could lead to a mis-match between in-application and external representation of text contents and thus bad behaviour.

@ogoffart

ogoffart commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

I think it is sad that you need to use nightly rust to get decent warning about non_exhaustive missing variable (non_exhaustive_omitted_patterns)

@ogoffart
ogoffart merged commit a98b2b2 into rust-windowing:master Sep 7, 2026
53 checks passed
@dhardy

dhardy commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

@ogoffart you can read my thoughts on that here. Even if it were stable, it wouldn't change my position on the changeset in this PR.

@kchibisov

Copy link
Copy Markdown
Member

Enum Ime should not be #[non_exhaustive] since ignoring variants could lead to a mis-match between in-application and external representation of text contents and thus bad behaviour.

While this is true, doing so would mean a breaking change for winit, since it's a semantic change that requires a semver bump, and we can still add those if we e.g. make those events opt-in, so non exhaustive doesn't really change much here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants