E532 Replace variant ImeRequest::Disable with fn disable_ime by dhardy · Pull Request #4426 · rust-windowing/winit · GitHub
[go: up one dir, main page]
Skip to content

Replace variant ImeRequest::Disable with fn disable_ime#4426

Open
dhardy wants to merge 5 commits intorust-windowing:masterfrom
dhardy:ime-fns
Open

Replace variant ImeRequest::Disable with fn disable_ime#4426
dhardy wants to merge 5 commits intorust-windowing:masterfrom
dhardy:ime-fns

Conversation

@dhardy
Copy link
Contributor
@dhardy dhardy commented Nov 25, 2025 •
edited
Loading
  • 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
kchibisov commented Nov 26, 2025

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 ch FF8 anges (not pushed yet): move winit::window::Ime* types to new winit_core::ime module, drop Ime prefix for these types.

@kchibisov
Copy link
Member
kchibisov commented Nov 26, 2025

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 •
edited
Loading

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
kchibisov commented Nov 26, 2025

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

Reviewers

@kchibisov kchibisov Awaiting requested review from kchibisov kchibisov is a code owner

@madsmtm madsmtm Awaiting requested review from madsmtm madsmtm is a code owner

@MarijnS95 MarijnS95 Awaiting requested review from MarijnS95 MarijnS95 is a code owner

@jackpot51 jackpot51 Awaiting requested review from jackpot51 jackpot51 is a code owner

@daxpedda daxpedda Awaiting requested review from daxpedda daxpedda is a code owner

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dhardy @kchibisov
0