winit icon indicating copy to clipboard operation
winit copied to clipboard

Replace variant ImeRequest::Disable with fn disable_ime

Open dhardy opened this issue 1 month ago • 6 comments

  • [ ] 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 avatar Nov 25 '25 13:11 dhardy

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.

dhardy avatar Nov 25 '25 14:11 dhardy

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

kchibisov avatar Nov 26 '25 06:11 kchibisov

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.

dhardy avatar Nov 26 '25 09:11 dhardy

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

kchibisov avatar Nov 26 '25 09:11 kchibisov

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

dhardy avatar Nov 26 '25 11:11 dhardy

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.

kchibisov avatar Nov 26 '25 12:11 kchibisov