Skip to content

Enable IME marked text on Windows and Linux#10122

Open
rikarazome wants to merge 6 commits into
warpdotdev:masterfrom
rikarazome:fix/enable-ime-marked-text-all-platforms
Open

Enable IME marked text on Windows and Linux#10122
rikarazome wants to merge 6 commits into
warpdotdev:masterfrom
rikarazome:fix/enable-ime-marked-text-all-platforms

Conversation

@rikarazome
Copy link
Copy Markdown

@rikarazome rikarazome commented May 5, 2026

Summary

  • Remove the macOS-only #[cfg(target_os = "macos")] guard on ImeMarkedText in RELEASE_FLAGS, enabling it for Windows and Linux release builds
  • Add ime_marked_text to the gui cargo feature so OSS/local builds also get IME composition text display by default

Context

The IME preedit (composition) text implementation is already fully platform-agnostic:

  • winit IME event handling dispatches SetMarkedText/ClearMarkedText on all platforms
  • grid_renderer draws marked text inline with underline decoration
  • Editor view handles marked text with proper Vim mode checks
  • Dogfood builds have had this enabled on all platforms via DOGFOOD_FLAGS

The only thing preventing it from working in release builds on Windows/Linux was the feature flag gate.

Test plan

  • Built and tested on Windows 11 with Japanese IME (Microsoft IME)
  • Confirmed preedit text (hiragana before kanji conversion) displays inline during composition
  • Confirmed text commits correctly after candidate selection
  • Linux IME testing (ibus/fcitx) — not tested locally, relying on CI and community validation

Fixes #7987
Fixes #3403
Related: #6891

rikarazome and others added 2 commits May 5, 2026 11:44
Remove the macOS-only cfg guard on ImeMarkedText in RELEASE_FLAGS.
The implementation (event handling, grid rendering, editor support)
is already platform-agnostic and has been active in dogfood builds
on all platforms.

Fixes warpdotdev#7987
Fixes warpdotdev#3403
Related: warpdotdev#6891

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Two changes to enable inline IME composition (preedit) text display
on all platforms:

1. Remove the macOS-only cfg guard on ImeMarkedText in RELEASE_FLAGS
   so release builds enable the flag on Windows and Linux.
2. Add ime_marked_text to the gui cargo feature so OSS/local builds
   also get IME support by default.

The underlying implementation (winit event handling, grid renderer,
editor marked text support) is already platform-agnostic and has been
validated in dogfood builds on all platforms.

Tested locally on Windows with Japanese IME - preedit text now
displays inline with underline decoration during composition.

Fixes warpdotdev#7987
Fixes warpdotdev#3403
Related: warpdotdev#6891

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@cla-bot
Copy link
Copy Markdown

cla-bot Bot commented May 5, 2026

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @rikarazome on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment @cla-bot check to trigger another check.

@github-actions github-actions Bot added the external-contributor Indicates that a PR has been opened by someone outside the Warp team. label May 5, 2026
@oz-for-oss
Copy link
Copy Markdown
Contributor

oz-for-oss Bot commented May 5, 2026

@rikarazome

I'm starting a first review of this pull request.

You can view the conversation on Warp.

I completed the review and no human review was requested for this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

oz-for-oss[bot]
oz-for-oss Bot previously requested changes May 5, 2026
Copy link
Copy Markdown
Contributor

@oz-for-oss oz-for-oss Bot left a comment

Choose a reason for hiding this comment

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

Overview

This PR enables the existing IME marked-text feature for release builds on Windows/Linux and includes the Cargo ime_marked_text feature in GUI builds.

Concerns

  • This is user-visible IME composition rendering behavior, but the PR description does not include screenshots or a video demonstrating the feature working end to end. For faster review, please upload screenshots or a video of the feature working end to end.

Verdict

Found: 0 critical, 1 important, 0 suggestions

Request changes

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

@rikarazome
Copy link
Copy Markdown
Author

rikarazome commented May 5, 2026

Screenshots

IME composition text working on Windows 11

Screenshot 2026-05-05 124222

Preedit text (「てすとてすて」) is displayed inline with underline decoration at the cursor position. The IME candidate window appears and text commits correctly after selection.

Note: The IME candidate window positioning could be improved (it overlaps the input area when the cursor is near the bottom of the terminal), but that is a pre-existing issue in update_ime_position and is out of scope for this PR.

@cla-bot check

@cla-bot cla-bot Bot added the cla-signed label May 5, 2026
@cla-bot
Copy link
Copy Markdown

cla-bot Bot commented May 5, 2026

The cla-bot has been summoned, and re-checked this pull request!

@rikarazome
Copy link
Copy Markdown
Author

@cla-bot check

@cla-bot
Copy link
Copy Markdown

cla-bot Bot commented May 5, 2026

The cla-bot has been summoned, and re-checked this pull request!

@rikarazome
Copy link
Copy Markdown
Author

C:/Program Files/Git/oz-review

@oz-for-oss
Copy link
Copy Markdown
Contributor

oz-for-oss Bot commented May 5, 2026

@rikarazome

I'm re-reviewing this pull request in response to a review request.

You can view the conversation on Warp.

I reviewed this pull request and requested human review from: @warpdotdev/oss-maintainers.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

@oz-for-oss oz-for-oss Bot dismissed their stale review May 5, 2026 04:30

Oz no longer requests changes for this pull request after the latest automated review.

Copy link
Copy Markdown
Contributor

@oz-for-oss oz-for-oss Bot left a comment

Choose a reason for hiding this comment

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

Overview

This PR enables the existing IME marked-text feature for Windows and Linux release builds and includes it in the GUI cargo feature for OSS/local graphical builds. The changed gates align with the platform-agnostic marked-text handling already present in the editor and terminal paths.

Concerns

  • No blocking correctness, security, or spec-alignment concerns found in the changed lines.

Verdict

Found: 0 critical, 0 important, 0 suggestions

Approve

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

@oz-for-oss oz-for-oss Bot requested review from a team and kevinchevalier and removed request for a team May 5, 2026 04:30
@abhishekp106 abhishekp106 requested review from abhishekp106 and removed request for kevinchevalier May 5, 2026 20:50
zerx-lab pushed a commit to zerx-lab/zap that referenced this pull request May 9, 2026
* fix(ime): enable IME marked text rendering on Windows

Mirrors the upstream fix from warpdotdev#10122 (still open since
2026-05-05) so OpenWarp users get IME preedit/composition rendering on
Windows without waiting for the upstream PR to land. Will be a no-op
delta when the next upstream sync absorbs warpdotdev#10122.

Two changes:

1. `crates/warp_features/src/lib.rs`: extend `RELEASE_FLAGS`'
   `FeatureFlag::ImeMarkedText` cfg from `target_os = "macos"` to
   `any(target_os = "macos", windows)`. The winit IME path on Windows
   already supports the `Ime::Preedit` event and dispatches
   `SetMarkedText`, but `set_marked_text_on_terminal` and similar
   render paths early-return when the feature flag is off.

2. `app/src/bin/oss.rs`: add `FeatureFlag::ImeMarkedText` to
   `additional_features` for Channel::Oss on macOS / Windows. Without
   this, dev/debug builds of `warp-oss` (which don't trip the
   `release_bundle` cfg gate that activates `RELEASE_FLAGS`) don't
   pick up the flag.

Result: Japanese / Chinese / Korean IME composition text now renders
inline in the terminal, search bar, and AI prompt in both release and
dev/debug builds of `warp-oss` on Windows.

Symptom this fixes: With Microsoft IME or Google Japanese Input,
candidate windows already showed up but the variable-length composition
string before commit was completely invisible. Confirmed fixed against
both IMEs on Windows 11.

Editor (CodeEditorView) preedit is a separate issue (no `SetMarkedText`
matcher there yet) and is not addressed here.

* Address CodeRabbit feedback on #63

- app/src/bin/oss.rs: merge `FeatureFlag` and `DEBUG_FLAGS` into the
  existing `warp_core::{...}` import block, drop the long-form
  `warp_core::features::FeatureFlag::ImeMarkedText` /
  `warp_core::features::DEBUG_FLAGS` paths in favor of the short
  imports, normalize the platform `cfg` to
  `any(target_os = "macos", target_os = "windows")`, and translate
  the explanatory comment to simplified Chinese to match repo style.
- crates/warp_features/src/lib.rs: same `cfg` normalization and
  comment translation around the `ImeMarkedText` flag.

No behavior change.
@Kyotani-Hub
Copy link
Copy Markdown

As a Japanese user on Windows 11, I'm directly impacted by this issue every day — typing Japanese (or any IME-driven language) in Warp is currently unusable because the pre-edit/composition text never appears inline. The candidate popup is the only visual feedback, which makes it impossible to compose text naturally.

This PR is +1/-3 lines and the underlying IME implementation is already done and dogfooded across platforms. Removing the macOS-only feature flag guard is all that stands between Windows/Linux users and a working IME.

Could this please get prioritized for review and merge? It would unblock a huge share of CJK users (issues #7987, #9012, #3403, #6891, #320, #6759, #8919, #9906, #9967 all converge here).

Thanks @rikarazome for the fix, and @abhishekp106 for any time you can spare on the review.

Comment thread app/Cargo.toml Outdated
# For the most part, including GUI code in headless builds is fine; the main use case is
# for things that pull in external dependencies.
gui = ["voice_input"]
gui = ["voice_input", "ime_marked_text"]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We shouldn't need ime_marked_text here since it's really only useful for things that pull in external dependencies, which marked text does not!

Copy link
Copy Markdown
Author

@rikarazome rikarazome May 20, 2026

Choose a reason for hiding this comment

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

Thanks for the review! Removed ime_marked_text from gui and added it to default instead, since it doesn't pull in any external dependencies.

FeatureFlag::Changelog,
FeatureFlag::CrashReporting,
// Marked text is currently only supported on MacOS.
#[cfg(target_os = "macos")]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since we've only locally tested on Windows, can we gate this on the target_os being macos or windows? I'm not confident that Linux works yet. There was a recent regression when enabling some IME-related functionality on Linux recently that needed to be reverted, so I want to be extra careful here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Gated ImeMarkedText in RELEASE_FLAGS with #[cfg(any(target_os = "macos", target_os = "windows"))] to exclude Linux. Understood about the regression in #11032.

@abhishekp106
Copy link
Copy Markdown
Contributor

I tested on my Windows machine and it generally seems to work ok! @rikarazome Can we tweak the PR to only enable marked text for Windows only? There was a recent regression (see #11032 for more context) on Linux and I think it needs more testing before getting enabled there.

Also since OS-related features are harder to test programmatically, could we get one more volunteers to test that their preferred input method editor works for them in a local build? Windows has a lot of different IMEs/setups and so I'd like to get some more coverage before enabling this more broadly. @Kyotani-Hub would you be willing and able?

Once someone else validates that things work for them and we scope this to Windows-only, I'd be happy to accept this PR. Sorry for the delay on my end in getting to your PR in a timely fashion.

rikarazome and others added 2 commits May 20, 2026 12:53
…in RELEASE_FLAGS

- Remove ime_marked_text from gui feature (no external deps)
- Add ime_marked_text to default feature list
- Gate ImeMarkedText in RELEASE_FLAGS to macos/windows only (exclude Linux due to warpdotdev#11032)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@rikarazome
Copy link
Copy Markdown
Author

Thanks for the review! Updated the PR:

I've also confirmed IME inline input works on my Windows build. Happy to adjust if needed.

Comment thread app/Cargo.toml
"directory_tab_colors",
"git_credential_refresh",
"configurable_context_window",
"ime_marked_text",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The inclusion of this flag in the defaults list will actually unconditionally enable marked text. Let's remove this. There is a fix ongoing to enable it when building locally in the case that you added it here to test it locally.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I tested removing ime_marked_text from default, but IME inline input stops working on Windows without it. The RELEASE_FLAGS cfg gate alone isn't sufficient — the Cargo feature is also needed because app/src/lib.rshas #[cfg(feature = "ime_marked_text")] gating the flag registration. How would you like me to handle this?

@abhishekp106
Copy link
Copy Markdown
Contributor

Of course! Just one more note about unconditionally removing ime_marked_text from the defaults array and then it's code complete.

@rikarazome
Copy link
Copy Markdown
Author

Hi @abhishekp106 , following up on the inline comment — I tested removing ime_marked_text from default, but IME inline input stops working on Windows without it. The Cargo feature is needed because app/src/lib.rs has #[cfg(feature = "ime_marked_text")] gating the flag registration.

Also, #11519 was just filed reporting the same issue (Korean IME on Windows) and was closed referencing this PR.

How would you like me to proceed?

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

Labels

cla-signed external-contributor Indicates that a PR has been opened by someone outside the Warp team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Japanese IME pre-conversion text not displayed on Windows Text input in multi-byte Japanese doesn't appear until confirmation

3 participants