Skip to content

Forward IME-committed text and guard redraws during composition (CJK lisp IME)#343

Open
junghan0611 wants to merge 1 commit into
dakra:mainfrom
junghan0611:fix/korean-ime-commit
Open

Forward IME-committed text and guard redraws during composition (CJK lisp IME)#343
junghan0611 wants to merge 1 commit into
dakra:mainfrom
junghan0611:fix/korean-ime-commit

Conversation

@junghan0611
Copy link
Copy Markdown

Hi @dakra,

Before anything else — thank you for ghostel. This project is genuinely precious to me, and I mean that.

A bit about who I am

I'm a Doom Emacs user and a CJK (Korean — Hangul) speaker, and I work daily in both GUI Emacs and -nw (terminal) Emacs. The -nw path matters to me as much as the GUI path. For years, getting Korean IME to behave reliably inside terminal-style buffers inside Emacs has been a constant low-grade pain. ghostel solved enough of that — and was clean enough underneath — that I rebuilt my terminal workflow around it.

I'm also a ghostty user, and honestly: I had been quietly thinking I might have to attempt this kind of project myself at some point. I'm very glad someone with your skill level got there first and built it properly. Truly grateful.

How I use ghostel

What this PR proposes

Two related changes, ~170 lines total, all in lisp/ghostel.el, zero native zig changes.

1. Forward IME-committed text to PTY for Emacs input methods

Some Emacs input methods commit text by directly inserting into the buffer (hangul-insert-character(self-insert-command 1) as a function call with last-command-event bound) instead of returning events from input-method-function. A function call bypasses [remap self-insert-command], so the character lands in the ghostel buffer but is never sent to the PTY — the next redraw erases it.

Fix: locally wrap input-method-function in ghostel buffers. Around the original IME call, observe whether point advanced; if so the IME committed text by buffer insertion. Capture that text, delete it from the buffer, and forward it to the PTY as UTF-8. The shell echoes it back through the normal redraw path, so the buffer ends up consistent without racing the redraw. IMEs that already return events (most quail packages) pass straight through. Restricted to PTY-forwarding modes (semi-char, char); line mode untouched.

2. Guard ghostel--redraw against running mid-composition

When a TUI streams output via ~256B PTY chunks while the user composes Korean via hangul2-input-method, syllables vanish — or 30+ byte chaos sequences (literally like 자갈ㅓㅏㅓㅏㅏㅏㅓㅏㅓㅏㅓ) get forwarded to the PTY.

Race: hangul2-input-method runs a read-key-sequence loop inside the IME wrapper, using the ghostel buffer as a scratch area; each key mutates the buffer via hangul-insert-characterself-insert-command. In parallel, agent output triggers ghostel--filter's immediate-redraw path, which calls ghostel--delayed-redraw synchronously; that body invokes the native ghostel--redraw to erase + reinsert the buffer from libghostty's VT grid. quail-overlay markers get invalidated mid-loop while inhibit-modification-hooks t silences quail's notifications, and the wrapper's buffer-substring then captures stale text.

Existing limit: ghostel--active-preedit-overlay only tracks x-preedit-overlay / pgtk-preedit-overlay — GUI native IME. Emacs-lisp IMEs (quail-overlay family: hangul, anthy, …) were never in scope; --capture-preedit-state / --restore-preedit-state don't see them.

Fix: a small predicate ghostel--ime-lisp-composing-p (a dynamic flag set by the IME wrapper + a live quail-overlay check) guards every code path that can end in ghostel--redraw rewriting the buffer:

  • The sync immediate-redraw branch in ghostel--filter — falls through to the bulk path during composition.
  • The body of ghostel--delayed-redraw — split into ghostel--delayed-redraw-body, with the guard living in the renamed wrapper. Catches every caller (theme sync, PTY filter immediate path, PTY filter bulk timer, M-x ghostel-force-redraw, window resize) in one place.

GUI native IME streaming behavior is intentionally unchanged — the predicate excludes x-preedit-overlay / pgtk-preedit-overlay. Line mode is unaffected. While the symptom that surfaced this is Korean hangul, the fix uses quail-overlay (Emacs core), so it generalizes to any lisp IME — Japanese anthy, Chinese, …

Detailed race-window analysis, invariant, and refactor guidance for future-you are documented inline as commentary blocks (the ;; Lisp IME composition guard section).

Why this is a draft

I don't send PRs the moment I write something. I keep the patch on a fork branch, pull upstream into it on a regular cadence, and use the result as my daily driver — that's how I shake bugs out. ghostel is your main project; it deserves to be treated that way.

By now this series has survived two upstream merges including the v0.30.0 release plus 118 commits, including refactors that touched the exact same area (9ff243f Centralize ghostel buffer terminal initialization, 22e535a Break out invalidation logic out of redraw) — with no merge conflicts and no behavioral regressions in daily use. That gives me reasonable confidence the placement is structural rather than coincidental.

That said, it stays as a draft until you weigh in. Things I'd love your judgement on:

  • Filesystem layout — keep inline in lisp/ghostel.el, or move to a new lisp/ghostel-ime.el. I left it inline because every call site lives in ghostel.el, but I have no strong preference.
  • Squash policy — currently 6 commits, preserving the diagnostic narrative (each commit body reads like a small race-condition write-up). Happy to fold into ~3 if you prefer fewer, larger commits.
  • Commentary tone — I included a lot of "if upstream restructures X, the guard's intent is …" notes inside the file. Useful, or noise?
  • Tests — the race is concurrent (PTY stream vs. IME compose loop), so I documented it as a commentary block rather than as ert. I can take a swing at a hypothesis-style test if you'd want one before merge.
  • Don't merge? — entirely fine. Please just say so and I'll keep it on my fork. The main reason I opened the PR at all is so other CJK users running streaming TUI agents inside ghostel can find this.

Branch / commits

  • Fork & branch: junghan0611/ghostel fix/korean-ime-commit
  • Foundation: 8d3320d Forward IME-committed text to PTY for Emacs input methods (2026-05-07)
  • Mode restriction follow-up: d9c5b11 Restrict IME commit forwarding to PTY-forwarding input modes
  • Guard series: 3b518a0 (helpers, no behavior change) → 83f110c (immediate-redraw) → 88cdb7a (delayed-redraw safety net) → db2484d (commentary fix)

다시 한 번, 진심으로 감사합니다.

@junghan0611

@emil-e
Copy link
Copy Markdown
Collaborator

emil-e commented May 28, 2026

Hi @junghan0611, I'm a co-maintainer with @dakra. Thanks for the PR. I will need some time to review this more carefully since I'm pretty keen on avoiding increased complexity in this particular area, but here are some initial thoughts:

  • The blocking of redraw makes sense but there's a lot of code inhibiting redraws now, or at least it feels like that. I wonder if you can instead hook this into ghostel--terminal-live-p or something like that.
  • Many of the comments are quite wordy and could use quite a bit of trimming down.

I do think proper support for Emacs IMEs is something we should have so would love to work with you to get this in. I don't have time right now for a more thorough review :)

@junghan0611
Copy link
Copy Markdown
Author

Hi @emil-e — thanks for the direction and the warm reception. Both points received.

I'll read ghostel--terminal-live-p and the surrounding gating helpers carefully before I touch anything — the goal is to see whether the lisp-IME predicate can be absorbed into existing terminal-state machinery cleanly, the way you suggested. If that consolidation works the new call site in ghostel--filter and the --delayed-redraw split both go away, and the diff shrinks a lot.

On the wordy comments — agreed, I'll trim to a single small block and move the race-window narrative to the commit body where it stops aging.

I'll work at my own pace and force-push to this branch when the rewrite is ready. No need to ping me — just take a look whenever you have bandwidth.

Thanks for the path.

junghan0611 pushed a commit to junghan0611/doomemacs-config that referenced this pull request May 28, 2026
…intainer feedback

PR submitted to upstream dakra/ghostel as draft (2026-05-28). Co-maintainer
@emil-e gave initial architectural direction:

- Fold lisp-IME guard into existing terminal-state machinery (e.g.
  `ghostel--terminal-live-p`) instead of adding a parallel predicate
  and splitting `ghostel--delayed-redraw`.
- Trim in-source commentary; move diagnostic narrative to commit body.

Promote PR + body + feedback into NEXT.md TOP so the refactor plan is
traceable from the repo root. Existing 2026-05-26 diagnostic record kept
as Background section.

Refs:
- PR:      dakra/ghostel#343
- @emil-e: dakra/ghostel#343 (comment)
- reply:   dakra/ghostel#343 (comment)
@dakra
Copy link
Copy Markdown
Owner

dakra commented May 28, 2026

Hi @junghan0611, thanks for the kind words (and @emil-e has to get all the praise for the glyph handling) ❤️

In addition to @emil-e's comment, I had a very quick peek.
It looks like there is a unrelated feature in there when native module loading fails. You can create a new PR with only that please (1 commit on top of main).

The ime changes look very small but also besides the 1 redraw inhibit line completely independent.
If we can somehow make it like @emil-e suggested that we don't directly need to call one of your new ime function,
this could be a completely separate ghostel-ime.el.

I'm also never a fan of adding hooks or changing someones Emacs just by loading a file.
So imho the 2 add-hook calls on the root scope should not be there.
How are other packages handling a situation like this normally?
Could we make a minor-mode and tell users that want it to just do something like
:hook (ghostel-mode . ghostel-ime-mode) or is it that every user always want this and we should just always activate / add-hook in the ghostel main function?

Anyway, you can force push on this branch.
Make a new PR, 1 commit, for the native module load feature,
and this PR, 1 commit, on top of main.

Thanks 🙏

@junghan0611 junghan0611 force-pushed the fix/korean-ime-commit branch from 2f996ba to 2a9fd7d Compare May 29, 2026 07:11
@junghan0611
Copy link
Copy Markdown
Author

Thanks both — this is the rewrite you asked for, force-pushed as a single commit on top of main.

What changed vs. the first draft

  • Split out into a new lisp/ghostel-ime.el. Core ghostel.el gains exactly one thing: a generic ghostel-inhibit-redraw-functions abnormal hook, checked once in ghostel--delayed-redraw. Core never calls any IME function directly — ghostel-ime registers its predicate on that hook. This is the consolidation @emil-e pointed at: the scattered redraw-inhibit collapsed to a single gate. (I kept it as its own defcustom rather than folding into ghostel--terminal-live-p, since copy-mode freeze and transient compose-defer read as different intents — happy to move it if you'd rather.)
  • No root-scope add-hook. It's now an opt-in buffer-local minor mode, ghostel-ime-mode. Loading the file changes nothing until a user opts in with :hook (ghostel-mode . ghostel-ime-mode) — same pattern as evil-ghostel. On your "how do other packages do this" question: this is the corfu-mode / flyspell-mode / electric-pair-mode convention — features that rewire input or environment are opt-in minor modes, not load-time hooks. GUI native IME is already handled by core, so only CJK lisp-IME users need this; making it always-on would be wrong.
  • Comments trimmed to a single short block; the race-window narrative now lives in the commit body where it won't age.
  • Native module load fallback dropped. I went to split it into its own PR as requested, but main already aborts via user-error in ghostel--load-module — it's equivalent (arguably stronger, since it checks file-exists-p rather than the ensure return value). So there's nothing to send; it just disappears from this branch.

Teststest/ghostel-ime-test.el, 8 ert cases (auto-picked up by make test / test-native). The full PTY-vs-compose race is environment-bound, so instead I pinned its two mechanisms as units: commit-forwarding (한/あ/中) and the redraw-defer hook (native tag). Also covers the minor-mode wiring, the list pass-through sentinel guard, and re-wrapping after an external state manager resets the translator.

Confirmed working in daily use with both pi and Claude Code — both heavy streaming TUIs, which is the exact race trigger this addresses.

Take your time — no rush.

junghan0611 pushed a commit to junghan0611/doomemacs-config that referenced this pull request May 29, 2026
- term-config.el: opt-in :hook (ghostel-mode . ghostel-ime-mode)
- packages.el: ghostel/evil-ghostel recipe restored to
  fix/korean-ime-commit (dakra/ghostel#343 rewrite, commit 2a9fd7d)
- NEXT.md: PR sent, awaiting maintainer review
@junghan0611 junghan0611 marked this pull request as ready for review May 29, 2026 08:29
@dakra
Copy link
Copy Markdown
Owner

dakra commented May 29, 2026

I haven't tested it but since the ghostel.el change is minimal and looks fine and ghostel-ime is completely independent and user opt-in I would give it a 👍 .

@emil-e you can check and merge it whenever you have time.

@emil-e
Copy link
Copy Markdown
Collaborator

emil-e commented May 29, 2026

I'll have a look as soon as I have the time!

@emil-e
Copy link
Copy Markdown
Collaborator

emil-e commented Jun 3, 2026

Sorry for the late review. It looks good to me, but needs a rebase. I have done quite some refactoring here to reduce complexity and your PR is unfortunately one of the casualties :)

@junghan0611 junghan0611 force-pushed the fix/korean-ime-commit branch from 2a9fd7d to f1a7060 Compare June 3, 2026 23:42
Some Emacs Lisp input methods, such as `hangul-input-method', commit
text by calling `self-insert-command' directly instead of returning
events from `input-method-function'.  A direct call bypasses ghostel's
key remapping, so the character lands in the buffer but is never sent
to the PTY and the next redraw erases it.  Separately, when a TUI
streams output while the user composes via a Quail-style method, the
renderer can rewrite the buffer mid-composition and corrupt the
in-flight syllable.

Add `ghostel-ime-mode', an opt-in buffer-local minor mode in a new
`ghostel-ime.el'.  When the input method commits by buffer insertion
the mode forwards the committed text to the PTY as UTF-8 and lets the
shell echo it back through the normal redraw path.

Core gains a single generic hook, `ghostel-inhibit-redraw-functions',
checked once in `ghostel--redraw-now': if any function returns
non-nil the redraw is deferred and rescheduled.  ghostel-ime registers
its composition predicate there, so core never calls into the IME code
directly.  GUI native preedit handling is unchanged.

Users opt in with `:hook (ghostel-mode . ghostel-ime-mode)'.  The
predicate keys on `quail-overlay' (Emacs core), so it generalizes to
any Lisp input method - Korean Hangul, Japanese, Chinese.

The mode also re-asserts the wrapper from `post-command-hook' so an
external input-method/state manager (Evil, for example) that restores
the raw translator without firing `input-method-activate-hook' does
not leave commits unforwarded.

Add ghostel-ime-test.el covering the deferral hook, the minor-mode
wiring, the list-sentinel guard, re-activation and external-reset
re-wrapping, CJK commit-forwarding, the quail-overlay probe, and
input-mode-switch survival.
@junghan0611 junghan0611 force-pushed the fix/korean-ime-commit branch from f1a7060 to e7dd8f3 Compare June 3, 2026 23:56
@junghan0611 junghan0611 changed the title [Draft] Forward IME-committed text and guard redraws during composition (CJK lisp IME) Forward IME-committed text and guard redraws during composition (CJK lisp IME) Jun 4, 2026
@junghan0611
Copy link
Copy Markdown
Author

Rebased onto current main.

Your delayed-redrawredraw-now refactor simplified this further: the IME guard is now a single generic gate inside ghostel--redraw-now, covering both the immediate and the timer redraw paths through one touch point.

Core stays IME-agnostic — ghostel.el only gains ghostel-inhibit-redraw-functions plus a small defer helper, and never calls into the IME code directly. All CJK / Lisp-IME behavior lives in the opt-in ghostel-ime.el minor mode. Default behavior is unchanged when the hook is empty.

Validation:

  • make test: pass
  • make checkdoc docquotes: pass
  • make test-native (IME cases): pass
  • Verified manually in terminal Emacs: Korean Hangul (quail) commit-forwarding and mid-composition redraw deferral both work as intended.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants