Skip to content

Conversation

@dhardy
Copy link
Contributor

@dhardy dhardy commented Nov 25, 2025

  • 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 the first part of #4412. It could be merged as-is or in concert with other changes.


Summary

Replace ImeRequest::Disable with fn disable_ime.

Motivation

It was documented that ImeRequest::Disable cannot fail, yet it returned a Result. This revision removes the API ambiguity.

@dhardy
Copy link
Contributor Author

dhardy commented Nov 25, 2025

Would you prefer to merge changes piecemeal (just this commit) or shall I push more IME changes into this PR?

The plan is roughly outlined here.

@kchibisov
Copy link
Member

I think I'll also look myself into it one more time and probably push some of the changes myself.

@dhardy
Copy link
Contributor Author

dhardy commented Nov 26, 2025

As you like.

I already made further changes (not pushed yet): move winit::window::Ime* types to new winit_core::ime module, drop Ime prefix for these types.

@kchibisov
Copy link
Member

If you split them into commits, it's fine with me. I can tune based on that.

@dhardy
Copy link
Contributor Author

dhardy commented Nov 26, 2025

Pushed.

My further plans are less concrete:

  • Split enable / update into separate requests. For the interface I think this makes sense but I'm hesistant because these ops share a lot of code in the backends.
  • Replace the error for update_ime with NotEnabled. Or possibly make it infallible ignoring errors.
  • Add ChangeCause parameter to update_ime
  • Implement From<ImeRequestData> for ImeEnableRequest to make usage a little easier, while keeping the existing path
  • Make hint and purpose required in enable_ime. However, this means that ImeRequestData cannot be used for both enable_ime and update_ime.
  • Revise documentation

@kchibisov
Copy link
Member

I think that update and enable should be together, since they are similar and the actual change is semantics, especially given that certain requests only apply on enable in some cases.

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.

2 participants