Skip to content

Comments

fix(tui): preserve full-width IME spaces#358

Closed
zemaj wants to merge 3 commits intojust-every:mainfrom
zemaj:fix/ime-fullwidth-spaces
Closed

fix(tui): preserve full-width IME spaces#358
zemaj wants to merge 3 commits intojust-every:mainfrom
zemaj:fix/ime-fullwidth-spaces

Conversation

@zemaj
Copy link

@zemaj zemaj commented Oct 28, 2025

Summary

  • trim only ASCII whitespace in composer normalization so full-width characters (e.g. U+3000) count as content
  • keep chat message construction from dropping full-width spaces when determining payload
  • add regression coverage in code-rs/tui/tests/fullwidth_ime.rs

Fixes #296

Testing

  • ./build-fast.sh (1.90.0)

Requesting preview validation from Windows/macOS users with full-width IME input to confirm the composer no longer drops U+3000 characters.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

https://github.com/just-every/code/blob/aaea0c0814e19beffc6c50f6be679fcaec3fc20a/code-rs/tui/src/chatwidget.rs#L3641-L3647
P1 Badge Preserve full-width chunks when parsing message text

The patch switches several trims to ASCII-only, but parse_message_with_images still drops segments that contain only full-width spaces. The checks that push text items (if !chunk.trim().is_empty()) run before the new normalization and use Unicode trim, so a message composed entirely of U+3000 characters will still yield an empty ordered_items vec and never reach the model. This means the commit does not actually fix #296 for the normal send path. These guards should mirror the new trim_matches(|c| c.is_ascii_whitespace()) logic (both here and in the trailing cursor < text.len() block) so full-width IME input is preserved when constructing the payload.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@zemaj
Copy link
Author

zemaj commented Oct 28, 2025

CI update:

Failing checks:

  • Build x86_64-unknown-linux-musl
  • Build aarch64-unknown-linux-musl
  • Build x86_64-apple-darwin

The first failure hits in the x86_64-unknown-linux-musl leg:

error[E0463]: can't find crate for core
note: the x86_64-unknown-linux-musl target may not be installed
(log)

This matches the existing workflow gap—preview-build is still booting Rust 1.89 without the musl targets. PR #356 switches the workflow to 1.90.0 and adds both musl targets, so once that lands we can rebase/merge and re-run this build; no code changes needed here.

Next steps: land #356 (and the publish-job gating in #359), then re-run the preview build for this branch.

@zemaj
Copy link
Author

zemaj commented Oct 28, 2025

CI red here matches #357: the preview workflow still runs Rust 1.89 without the musl targets, so the build dies with error[E0463]: can't find crate for core. After #356 lands (toolchain/target update) we can re-run this job and expect it to pass. No code changes needed.

@zemaj
Copy link
Author

zemaj commented Oct 28, 2025

Heads-up: the previous toolchain fix in #356 was closed; replacement PR #364 is open and once it merges we’ll rerun preview-build to clear the musl/darwin failures here.

@zemaj
Copy link
Author

zemaj commented Oct 28, 2025

Preview Build rerun triggered after #364 merged; watching for green checks.

@zemaj
Copy link
Author

zemaj commented Oct 28, 2025

Fresh Preview Build queued with updated toolchain after #364 merged; watching for green checks.

@zemaj
Copy link
Author

zemaj commented Oct 28, 2025

Preview Build rerun queued with Rust 1.90.0 plus MUSL targets after #370 merged. Tracking run: https://github.com/just-every/code/actions/runs/18870453662

@zemaj zemaj force-pushed the main branch 2 times, most recently from 804b5ad to b7927a2 Compare October 28, 2025 23:57
@zemaj zemaj closed this in #364 Oct 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

When inputting in Chinese, it automatically adds a space at the end of the current input

1 participant