fix: resolve input, focus, persistence, and terminal regressions - #83
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c637bd40fa
ℹ️ 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".
| let input_top = match state.mode() { | ||
| ThemeSettingsMode::Theme => THEME_FILTER_TOP, | ||
| ThemeSettingsMode::Settings => SETTINGS_TOP, | ||
| }; | ||
| CaretPt { | ||
| x: card_x + 20.0, | ||
| y: card_top + input_top, |
There was a problem hiding this comment.
On macOS this always anchors at the fixed top-left of the settings section. It never advances with the theme/settings search query or live preedit, and when IME is used to edit fields such as Font Family or Background Image while search is inactive, the real input is the selected settings row rather than SETTINGS_TOP. Consequently the candidate window is detached from the visible caret for every theme-settings text-entry mode; derive both coordinates from the active section/row and current buffer plus preedit.
Useful? React with 👍 / 👎.
| let card = layout.cards.iter().find(|card| card.id == session.card)?; | ||
| let rect = card.name_line; | ||
| Some(( | ||
| PhysicalPosition::new(rect.x as i32, rect.y as i32), | ||
| PhysicalSize::new(1, rect.h.max(1)), |
There was a problem hiding this comment.
During a macOS sidebar rename, this pins the IME candidate window to the left edge of name_line regardless of the existing name, appended text, or live preedit. Since the rendered rename caret is after buffer + preedit, the candidate window remains at the beginning of the field while the user types; calculate the horizontal caret position from that displayed text, with the same clipping behavior as the sidebar renderer.
Useful? React with 👍 / 👎.
| let layout = PaletteCardLayout::new(pane, snapshot); | ||
| CaretPt { | ||
| x: layout.card_x + CARD_PAD_H + 22.0 + query_chars as f64 * INPUT_FONT_ADVANCE, | ||
| y: layout.card_top + 13.0, |
There was a problem hiding this comment.
For a long command-palette query or remote endpoint on macOS, query_chars * INPUT_FONT_ADVANCE grows without regard to the finite card width. Once the query exceeds roughly the visible row capacity, the returned caret passes the card and eventually the window edge even though the label is clipped, causing the candidate window to appear far from the visible input. Clamp or scroll the anchor using the query row's actual content bounds.
Useful? React with 👍 / 👎.
…ings Ten of twelve findings from the 2026-09-10 static analysis (N01-N12); N08 and N10 are rejected as Ghostty parity (key_encode.zig escape-encodes under any non-zero kitty flag and sends IME/unidentified-key text raw). - N01 secure input: decide from app-level focus (`os_focused` after the loss), so an outgoing window's `Focused(false)` that follows the successor's `Focused(true)` no longer releases Secure Keyboard Entry. - N02 ipc: `noa.sendText` reports a dropped (queue full) or sink-less (disconnected) input as an error instead of `Ok`. - N03 mouse: the left press captures its pane; motion and release of that gesture route to it across split dividers, with cancellation on focus loss, pane close, and pane move. - N04 config writer: after a font-family save, re-parse with includes expanded and, when an include's family still heads the list, rewrite the key as reset + new primary + surviving fallbacks. - N05 favorites: share session.rs's per-process/per-call staging file via a new `atomic_write` module instead of a fixed `favorites.tmp`. - N06 grid: `clear_scrollback` shares `ED 3`'s placement collapse so live Kitty images keep their row when only history is cleared. - N07 grid: with DECOM off, CUP/CHA/HPA and the DECSLRM home are bounded by the screen, not the left/right margins (Ghostty `setCursorPos`). - N09 kitty: a composing macOS Option is not Alt for the escape decision; the composed text passes through, report-all keeps the modifier bit and the associated text. - N11 kitty: physical keypad keys use their dedicated codes (KP_Enter is 57414 even though winit's logical key is Enter). - N12 ime: the candidate window anchors to the owning modal's input row (palette / remote UI / title prompt / theme settings / search prompt / sidebar rename) instead of the terminal cursor behind the card.
Absolute placement outside horizontal margins must not make relative motion reverse direction or scroll unrelated cells during wrapping, LF, or RI. Use the rendered search status width and the palette's visible rows to keep IME candidate anchors aligned with the active input field. Add regressions for boundary movement, clipped search text, and short palette panes with mixed row types.
c637bd4 to
bafa18c
Compare
1 participant
Fix regressions in split-pane input routing, terminal margins, persistence, and modal IME placement.
noa.sendTextwhen the input queue rejects bytes or the pane is disconnected. Queue acceptance still does not guarantee that the child has consumed the input.Validation
cargo test --workspace --no-fail-fast— 2,653 passed, 15 ignored on macOS outside the sandbox. The unrestricted run also passed the shared-memory and socket tests that the sandbox denied.cargo build --workspace— passed.cargo fmt --all --check— passed.git diff --check— passed.Compatibility and remaining manual checks
No persistence format or IPC schema changes. Concurrent saves remain whole-file, last-writer-wins replacements. Reverting the code requires no data migration.
Native focus transitions, cross-pane mouse gestures, and live macOS IME candidate windows have not been manually exercised or recorded; verification covers the event policies and layout calculations. Proportional-font palette/title caret X positions use a font-advance estimate.