diff --git a/crates/noa-app/src/app/event_loop.rs b/crates/noa-app/src/app/event_loop.rs index 6bf4294d..68f5e9eb 100644 --- a/crates/noa-app/src/app/event_loop.rs +++ b/crates/noa-app/src/app/event_loop.rs @@ -30,6 +30,34 @@ fn shared_modifiers_after_focus_loss( } } +/// The app-level OS focus after `window_id` reports `Focused(false)`. When +/// macOS switches between two of our own windows the incoming window's +/// `Focused(true)` can land first and already repoint `os_focused`; the +/// outgoing window's loss must then leave it alone. Every consumer that +/// needs "is this *app* still frontmost" (Secure Keyboard Entry above all) +/// derives it from this result, never from the per-window boolean. +fn os_focused_after_focus_loss( + os_focused: Option, + window_id: WindowId, +) -> Option { + if os_focused == Some(window_id) { + None + } else { + os_focused + } +} + +/// The pane a button event routes to: the pane holding the live left-button +/// capture first, then the pane under the pointer, then the focused pane +/// (for events with no pointer position at all). +fn mouse_gesture_pane( + captured: Option, + hovered: Option, + focused: PaneId, +) -> PaneId { + captured.or(hovered).unwrap_or(focused) +} + impl App { /// Pane-dnd P1-1 remediation (`docs/specs/pane-dnd.md` L2(e)): re-resolve /// a pane-scoped `UserEvent`'s window at *receive* time rather than @@ -659,6 +687,7 @@ impl ApplicationHandler for App { // modifier press, so nothing is lost. if let Some(state) = self.windows.get_mut(&window_id) { state.modifiers = ModifiersState::empty(); + state.key_modifiers.clear(); } self.end_copy_mode_for_window(window_id); // Only clear if this window is the one we recorded as focused — @@ -668,9 +697,11 @@ impl ApplicationHandler for App { let was_input_target = self.os_focused == Some(window_id); self.modifiers = shared_modifiers_after_focus_loss(self.modifiers, was_input_target); - if was_input_target { - self.os_focused = None; - } + self.os_focused = os_focused_after_focus_loss(self.os_focused, window_id); + // A left-button gesture captured by one of this window's panes + // can't complete once focus is gone (the release may never + // arrive): drop it now so the pane isn't left "pressed". + self.cancel_mouse_capture(window_id); self.finish_active_split_drag(window_id); // Cancel an in-flight overview pane drag when its host window // loses focus (Cmd-Tab / native tab switch mid-drag): the @@ -712,10 +743,15 @@ impl ApplicationHandler for App { self.report_focus_event(window_id, false); // Release Secure Keyboard Entry while backgrounded so it never // blocks key input to the rest of the system; a matching - // `Focused(true)` (including switching between our own windows) - // restores it. - self.secure_input - .on_focus_change(false, &mut crate::secure_input::CarbonSecureInput); + // `Focused(true)` restores it. The decision is made from the + // *app-level* focus computed above, not this window's boolean: + // when another of our windows already took focus (its + // `Focused(true)` arrived first) the app is still frontmost + // and the protection must stay up (N01). + self.secure_input.on_focus_change( + self.os_focused.is_some(), + &mut crate::secure_input::CarbonSecureInput, + ); if let Some(state) = self.windows.get(&window_id) { state.window.request_redraw(); } @@ -826,6 +862,22 @@ impl ApplicationHandler for App { WindowEvent::Ime(event) => self.on_ime_event(window_id, event), WindowEvent::KeyboardInput { event, .. } => { let pressed = event.state == ElementState::Pressed; + // Pair before any modal/shortcut early return so every + // release also retires the physical key's classification. + let event_alt_sends_esc = !cfg!(target_os = "macos") + || event.text.as_deref().is_none_or(str::is_empty) + || event.text.as_deref() != event.text_with_all_modifiers(); + let alt_sends_esc = + self.windows + .get_mut(&window_id) + .map_or(event_alt_sends_esc, |state| { + state.key_modifiers.alt_sends_esc( + event.physical_key, + pressed, + event.repeat, + event_alt_sends_esc, + ) + }); if pressed { // NOA_LATENCY_TRACE t0: winit key-event receipt, before // any routing — this is the earliest app-side timestamp @@ -1079,13 +1131,6 @@ impl ApplicationHandler for App { let (app_cursor_keys, app_keypad, kitty_flags, modify_other_keys) = self.key_encode_modes(window_id); let unmodified_key = event.key_without_modifiers(); - // On macOS, Option only acts as Alt when winit stripped its - // composition per `macos-option-as-alt` — i.e. the delivered - // text differs from the text with every modifier applied. - // Otherwise the composed character must pass through with no - // ESC prefix. - let alt_sends_esc = !cfg!(target_os = "macos") - || event.text.as_deref() != event.text_with_all_modifiers(); let bytes = input::encode_key_with_modes( &event.logical_key, Some(&unmodified_key), @@ -1406,7 +1451,22 @@ impl App { return; } - let Some((pane_id, cell)) = self.pane_cell_at_position(window_id, position, metrics) else { + // A left press captured a pane: this motion belongs to it (the cell is + // computed against *its* rect, clamped at the edges), not to whatever + // pane the pointer is over now. That keeps a selection or SGR drag + // that crosses a divider extending in the pane that saw the press, + // and lets that pane — not the neighbour — receive the release (N03). + let captured = self + .windows + .get(&window_id) + .and_then(|state| state.mouse_capture_pane); + let resolved = match captured { + Some(pane_id) => self + .pane_cell_in(window_id, pane_id, position, metrics) + .map(|cell| (pane_id, cell)), + None => self.pane_cell_at_position(window_id, position, metrics), + }; + let Some((pane_id, cell)) = resolved else { if let Some(state) = self.windows.get_mut(&window_id) { state.last_mouse_pane = None; } @@ -1462,9 +1522,17 @@ impl App { pub(super) fn on_cursor_left(&mut self, window_id: WindowId) { if let Some(state) = self.windows.get_mut(&window_id) { state.last_mouse_point = None; - state.last_mouse_pane = None; - for surface in state.surfaces.values_mut() { - surface.last_mouse_cell = None; + state.last_mouse_physical_position = None; + // A captured gesture survives the pointer leaving the window: the + // release still arrives here (macOS delivers mouse-up to the + // window that saw mouse-down) and must find the pane and its last + // cell intact. + let captured = state.mouse_capture_pane; + state.last_mouse_pane = captured; + for (pane_id, surface) in &mut state.surfaces { + if Some(*pane_id) != captured { + surface.last_mouse_cell = None; + } } } self.update_sidebar_button_hover(window_id, None); @@ -1640,15 +1708,29 @@ impl App { } } - let pane_id = self - .windows - .get(&window_id) - .and_then(|state| state.last_mouse_pane) - .or_else(|| self.windows.get(&window_id).map(|state| state.focused_pane)); + let pane_id = self.windows.get(&window_id).map(|tab| { + mouse_gesture_pane( + tab.mouse_capture_pane, + tab.last_mouse_pane, + tab.focused_pane, + ) + }); let Some(pane_id) = pane_id else { return; }; + // The left button captures the pane it pressed in until its release + // (see `on_cursor_moved`); the release always goes to the capturing + // pane, so a press in A and a release over B still clears A's + // pressed/drag state instead of handing B a release it never saw the + // press for. + if button == MouseButton::Left + && state == ElementState::Pressed + && let Some(tab) = self.windows.get_mut(&window_id) + { + tab.mouse_capture_pane = Some(pane_id); + } + if button == MouseButton::Left && state == ElementState::Pressed { self.focus_pane(window_id, pane_id); } @@ -1687,6 +1769,9 @@ impl App { } } } + if button == MouseButton::Left && state == ElementState::Released { + self.release_mouse_capture(window_id); + } return; } @@ -1727,6 +1812,9 @@ impl App { }) .unwrap_or(SelectionGesture::None); self.apply_selection_gesture(window_id, pane_id, gesture); + if state == ElementState::Released { + self.release_mouse_capture(window_id); + } } pub(super) fn on_mouse_wheel(&mut self, window_id: WindowId, delta: MouseScrollDelta) { @@ -1842,6 +1930,10 @@ impl App { } Ime::Enabled | Ime::Disabled => self.modal_preedit = None, } + // The candidate window follows the modal's caret (N12): re-anchor + // on every composition step so it tracks the typed text rather + // than the terminal cursor behind the card. + self.update_focused_ime_cursor_area(window_id); self.request_window_redraw(window_id); return; } @@ -2069,3 +2161,72 @@ mod window_modifier_tests { ); } } + +#[cfg(test)] +mod focus_loss_tests { + use super::*; + use crate::secure_input::{SecureInput, SecureInputBackend}; + + struct Recording(Vec); + impl SecureInputBackend for Recording { + fn set_enabled(&mut self, enabled: bool) { + self.0.push(enabled); + } + } + + fn window(id: u64) -> WindowId { + WindowId::from(id) + } + + // N01: `B Focused(true)` → `A Focused(false)` (macOS reorders the pair + // when switching between our own windows). A's loss must not read as + // "the app lost focus": `os_focused` still points at B. + #[test] + fn outgoing_window_loss_keeps_app_focus_when_successor_already_focused() { + let (a, b) = (window(1), window(2)); + let os_focused = Some(b); + assert_eq!(os_focused_after_focus_loss(os_focused, a), Some(b)); + assert_eq!(os_focused_after_focus_loss(Some(a), a), None); + assert_eq!(os_focused_after_focus_loss(None, a), None); + } + + // The Secure Keyboard Entry state driven from that app-level focus: + // `A true → B true → A false` leaves the protection up; only `B false` + // (the last window) releases it. + #[test] + fn secure_input_survives_intra_app_window_switch() { + let (a, b) = (window(1), window(2)); + let mut secure = SecureInput::new(); + let mut backend = Recording(Vec::new()); + let mut os_focused = Some(a); + secure.toggle(os_focused.is_some(), &mut backend); + assert_eq!(backend.0, [true]); + + // B gains focus first … + os_focused = Some(b); + secure.on_focus_change(os_focused.is_some(), &mut backend); + // … then A's loss arrives: still frontmost, nothing released. + os_focused = os_focused_after_focus_loss(os_focused, a); + secure.on_focus_change(os_focused.is_some(), &mut backend); + assert_eq!( + backend.0, + [true], + "A's stale loss must not release the switch" + ); + + // B's loss is the real backgrounding. + os_focused = os_focused_after_focus_loss(os_focused, b); + secure.on_focus_change(os_focused.is_some(), &mut backend); + assert_eq!(backend.0, [true, false]); + } + + // N03: a live left-button capture wins over the pane under the pointer, + // so the release of a press in A that ends over B still routes to A. + #[test] + fn captured_pane_outranks_hovered_pane_for_button_events() { + let (a, b, focused) = (PaneId::new(1), PaneId::new(2), PaneId::new(3)); + assert_eq!(mouse_gesture_pane(Some(a), Some(b), focused), a); + assert_eq!(mouse_gesture_pane(None, Some(b), focused), b); + assert_eq!(mouse_gesture_pane(None, None, focused), focused); + } +} diff --git a/crates/noa-app/src/app/input_ops/ime.rs b/crates/noa-app/src/app/input_ops/ime.rs index 9dc68802..ecba1f4b 100644 --- a/crates/noa-app/src/app/input_ops/ime.rs +++ b/crates/noa-app/src/app/input_ops/ime.rs @@ -1,5 +1,14 @@ use super::super::*; +fn search_prompt_ime_caret_col( + buffer: &str, + preedit: &str, + search: &noa_grid::SearchState, + cols: u16, +) -> u16 { + noa_render::search_prompt_caret_col(&format!("{buffer}{preedit}"), search, cols) +} + impl App { pub(in crate::app) fn modal_ime_target(&self, window_id: WindowId) -> Option { if self @@ -105,10 +114,9 @@ impl App { } } - // #TODO(agent): while a modal (search prompt / palette / rename) owns the - // composition, the candidate window still anchors to the terminal cursor - // below — it should anchor to the modal's caret instead (needs the modal - // card's pixel geometry here). + /// Tell the OS where the composition caret is, so the IME candidate + /// window opens beside it. While a modal owns the composition that is + /// the modal's input row (N12); otherwise the terminal cursor. pub(in crate::app) fn update_focused_ime_cursor_area(&self, window_id: WindowId) { let Some(gpu) = self.gpu.as_ref() else { return; @@ -119,17 +127,157 @@ impl App { let Some(surface) = state.focused_surface() else { return; }; + let metrics = gpu.fonts.get(state.font_px).metrics(); + if let Some((position, size)) = self.modal_ime_caret_area(window_id, state, metrics) { + state.window.set_ime_cursor_area(position, size); + return; + } let cursor = { let terminal = surface.terminal.lock(); terminal.active().cursor }; update_ime_cursor_area( &state.window, - gpu.fonts.get(state.font_px).metrics(), + metrics, cursor.x, cursor.y, surface.rect, self.padding, ); } + + /// The caret area (window-relative physical pixels) of the modal that + /// owns `window_id`'s composition, `None` when no modal does or the + /// owner has no text field (confirm dialog). The card geometry comes + /// from the same constants the AppKit builders use; the live preedit is + /// counted into the text length so the anchor tracks the composition. + fn modal_ime_caret_area( + &self, + window_id: WindowId, + state: &WindowState, + metrics: noa_font::Metrics, + ) -> Option<(PhysicalPosition, PhysicalSize)> { + let target = self.modal_ime_target(window_id)?; + let preedit_chars = self.modal_preedit_for(window_id, target).chars().count(); + let focused = state.focused_surface()?; + let scale = state.window.scale_factor(); + let pane = crate::macos_overlay::PaneRectPt::from_px( + focused.rect.x, + focused.rect.y, + focused.rect.w, + focused.rect.h, + scale, + ); + let caret_px = |caret: crate::macos_overlay::CaretPt| { + ( + PhysicalPosition::new( + ((pane.x + caret.x) * scale).round().max(0.0) as i32, + ((pane.y + caret.y) * scale).round().max(0.0) as i32, + ), + PhysicalSize::new( + (caret.w * scale).ceil().max(1.0) as u32, + (caret.h * scale).ceil().max(1.0) as u32, + ), + ) + }; + match target { + ModalImeTarget::ConfirmDialog => None, + ModalImeTarget::SearchPrompt => { + let session = self.search_prompt.as_ref()?; + let surface = state.surfaces.get(&session.pane_id)?; + let col = search_prompt_ime_caret_col( + session.prompt.buffer(), + self.modal_preedit_for(window_id, target), + &surface.terminal.lock().active().search, + surface.grid_size.cols, + ); + Some(ime_cursor_area(metrics, col, 0, surface.rect, self.padding)) + } + ModalImeTarget::CommandPalette => { + let session = self.command_palette.as_ref()?; + let snapshot = + command_palette_snapshot(&self.keybinds, &session.palette, |command| { + self.command_is_enabled(window_id, command) + }); + Some(caret_px(crate::macos_overlay::palette_query_caret( + pane, + &snapshot, + snapshot.query.chars().count() + preedit_chars, + ))) + } + ModalImeTarget::RemoteUi => { + let (snapshot, _) = self.remote_ui_snapshot(window_id)?; + Some(caret_px(crate::macos_overlay::palette_query_caret( + pane, + &snapshot, + self.remote_ui_input_chars() + preedit_chars, + ))) + } + ModalImeTarget::TabTitlePrompt => { + let chars = self + .tab_title_prompt + .as_ref() + .map_or(0, |session| session.buffer.chars().count()); + Some(caret_px(crate::macos_overlay::title_prompt_caret( + pane, + chars + preedit_chars, + ))) + } + ModalImeTarget::ThemeSettings => { + let session = self.theme_settings.as_ref()?; + Some(caret_px(crate::macos_overlay::theme_settings_caret( + pane, + &session.state, + ))) + } + ModalImeTarget::SidebarRename => { + // The renamed card's name row, from the same layout the + // sidebar draws and hit-tests with. + let session = self.sidebar_rename.as_ref()?; + let inset = self.window_sidebar_inset_px(window_id); + let bounds = self.sidebar_layout_bounds(window_id, inset); + let windows = self.session_windows_for_window(window_id); + let ids = self.session_store.ordered_ids_for_windows(&windows); + let layout = + self.sidebar_metrics(window_id) + .layout(bounds, &ids, state.sidebar_scroll); + 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)), + )) + } + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn search_prompt_ime_caret_follows_the_drawn_status_suffix() { + let mut search = noa_grid::SearchState::default(); + search.set_query( + "needle".to_string(), + Vec::new(), + noa_grid::SearchAnchor::Backward(noa_grid::SelectionPoint::new(0, 0)), + ); + assert_eq!( + search_prompt_ime_caret_col(&"a".repeat(30), &"b".repeat(10), &search, 80), + 68 + ); + assert_eq!(search_prompt_ime_caret_col("a", "", &search, 80), 68); + assert_eq!( + search_prompt_ime_caret_col(&"日".repeat(100), "本", &search, 80), + 68 + ); + assert_eq!(search_prompt_ime_caret_col("a", "", &search, 5), 0); + assert_eq!(search_prompt_ime_caret_col("a", "", &search, 0), 0); + assert_eq!( + search_prompt_ime_caret_col("", "日本", &noa_grid::SearchState::default(), 80), + 75 + ); + } } diff --git a/crates/noa-app/src/app/input_ops/pointer.rs b/crates/noa-app/src/app/input_ops/pointer.rs index d857136d..69a9360e 100644 --- a/crates/noa-app/src/app/input_ops/pointer.rs +++ b/crates/noa-app/src/app/input_ops/pointer.rs @@ -159,18 +159,72 @@ impl App { Some(HitTarget::Pane(pane_id)) => pane_id, Some(HitTarget::Divider) | None => return None, }; - let surface = state.surfaces.get(&pane_id)?; + let cell = self.pane_cell_in(window_id, pane_id, position, metrics)?; + Some((pane_id, cell)) + } + + /// The grid cell of `pane_id` under a window-relative `position`, with no + /// hit test: a position outside the pane's rect clamps to its edge cells. + /// This is how a captured drag keeps reporting cells of the pane that saw + /// the press after the pointer has crossed a divider. `None` only when + /// the pane no longer exists in `window_id`. + pub(in crate::app) fn pane_cell_in( + &self, + window_id: WindowId, + pane_id: PaneId, + position: PhysicalPosition, + metrics: noa_font::Metrics, + ) -> Option { + let surface = self.windows.get(&window_id)?.surfaces.get(&pane_id)?; let local_x = position.x - f64::from(surface.rect.x); let local_y = position.y - f64::from(surface.rect.y); - let cell = mouse::physical_position_to_grid_point( + Some(mouse::physical_position_to_grid_point( local_x, local_y, metrics.cell_w, metrics.cell_h, surface.grid_size, self.padding, - ); - Some((pane_id, cell)) + )) + } + + /// Re-hit-test the physical pointer after delivering mouse-up to the + /// captured pane. This updates routing without synthesizing mouse motion. + pub(in crate::app) fn release_mouse_capture(&mut self, window_id: WindowId) { + let hovered = (|| { + let state = self.windows.get(&window_id)?; + let position = state.last_mouse_physical_position?; + let metrics = self.gpu.as_ref()?.fonts.get(state.font_px).metrics(); + self.pane_cell_at_position(window_id, position, metrics) + })(); + if let Some(state) = self.windows.get_mut(&window_id) { + state.mouse_capture_pane = None; + state.last_mouse_pane = hovered.map(|(pane_id, _)| pane_id); + for (pane_id, surface) in &mut state.surfaces { + surface.last_mouse_cell = hovered + .filter(|(hovered_pane, _)| hovered_pane == pane_id) + .map(|(_, cell)| cell); + } + } + self.sync_hover_link(window_id); + } + + /// Abandon the live left-button capture in `window_id` (focus loss): the + /// capturing pane's pressed-button and selection-drag state are cleared + /// as if its release had arrived, so a later motion can't keep extending + /// a selection or report a phantom button-held drag to a tracking TUI. + pub(in crate::app) fn cancel_mouse_capture(&mut self, window_id: WindowId) { + let Some(state) = self.windows.get_mut(&window_id) else { + return; + }; + let Some(pane_id) = state.mouse_capture_pane.take() else { + return; + }; + if let Some(surface) = state.surfaces.get_mut(&pane_id) { + surface.pressed_mouse_button = None; + let _ = surface.mouse_selection.left_released(); + } + self.release_mouse_capture(window_id); } /// The Cmd+hover link under the mouse in `window_id`'s focused-under- diff --git a/crates/noa-app/src/app/ipc.rs b/crates/noa-app/src/app/ipc.rs index c7bf4871..d251e6e4 100644 --- a/crates/noa-app/src/app/ipc.rs +++ b/crates/noa-app/src/app/ipc.rs @@ -398,7 +398,12 @@ impl App { self.mark_pane_paste_input(window_id, pane_id); } self.snap_pane_viewport_to_bottom(window_id, pane_id); - self.write_pane_pty_bytes(window_id, pane_id, bytes); + // The queue outcome is the reply: a full input budget or a + // gone sink must surface to the automation client, which + // otherwise proceeds as if the text had been accepted + // (N02). `write_pane_pty_bytes` only logs those. + let result = self.queue_pane_pty_bytes(window_id, pane_id, bytes); + ipc_input_queue_result(result)?; } Ok(IpcActionReply::Ok) } @@ -578,6 +583,26 @@ fn apply_attach_grid_first_resize( dispatch_pty_resize(new_size) } +/// Map a pty input queue outcome onto the `noa.sendText` reply. `Queued` and +/// `Deferred` are both acceptance (the queue owns the bytes and will write +/// them); `Dropped` means the pane's input budget is exhausted because the +/// foreground program isn't reading its tty, and `Disconnected` that the +/// pane has no live sink (writer thread or remote transport gone). Neither +/// of the last two may be reported as success. "Accepted by the queue" is +/// still not "consumed by the child" — that guarantee doesn't exist. +fn ipc_input_queue_result( + result: crate::io_thread::QueueInputResult, +) -> Result<(), noa_ipc::IpcError> { + use crate::io_thread::QueueInputResult; + match result { + QueueInputResult::Queued | QueueInputResult::Deferred => Ok(()), + QueueInputResult::Dropped => Err(noa_ipc::IpcError::Internal( + "pty input queue is full: the foreground program is not reading its tty".to_string(), + )), + QueueInputResult::Disconnected => Err(noa_ipc::IpcError::PaneClosed), + } +} + #[cfg(test)] mod attach_tests { use super::*; @@ -618,3 +643,25 @@ mod attach_tests { ); } } + +#[cfg(test)] +mod send_text_reply_tests { + use super::*; + use crate::io_thread::QueueInputResult; + + // N02: a rejected or sink-less queue must not turn into `Ok` for the + // automation client; only actual acceptance does. + #[test] + fn send_text_reports_queue_rejection_instead_of_success() { + assert!(ipc_input_queue_result(QueueInputResult::Queued).is_ok()); + assert!(ipc_input_queue_result(QueueInputResult::Deferred).is_ok()); + assert!(matches!( + ipc_input_queue_result(QueueInputResult::Dropped), + Err(noa_ipc::IpcError::Internal(message)) if message.contains("queue is full") + )); + assert!(matches!( + ipc_input_queue_result(QueueInputResult::Disconnected), + Err(noa_ipc::IpcError::PaneClosed) + )); + } +} diff --git a/crates/noa-app/src/app/lifecycle.rs b/crates/noa-app/src/app/lifecycle.rs index 18382d63..e931a085 100644 --- a/crates/noa-app/src/app/lifecycle.rs +++ b/crates/noa-app/src/app/lifecycle.rs @@ -608,10 +608,12 @@ impl App { focused_pane: initial_pane, surfaces, last_mouse_pane: Some(initial_pane), + mouse_capture_pane: None, last_mouse_point: None, last_mouse_physical_position: None, active_split_drag: None, modifiers: ModifiersState::empty(), + key_modifiers: input::KeyModifierState::default(), occluded: false, title: "Noa".to_string(), proxy_icon_cwd: None, @@ -1372,6 +1374,9 @@ impl App { .surfaces .get(&pane_id) .and_then(|surface| surface.scrollback_key.clone()); + if state.mouse_capture_pane == Some(pane_id) { + state.mouse_capture_pane = None; + } if let Some(mut surface) = state.surfaces.remove(&pane_id) { surface.shutdown(); } diff --git a/crates/noa-app/src/app/quick_terminal.rs b/crates/noa-app/src/app/quick_terminal.rs index 7511bcfb..64603617 100644 --- a/crates/noa-app/src/app/quick_terminal.rs +++ b/crates/noa-app/src/app/quick_terminal.rs @@ -821,10 +821,12 @@ impl App { focused_pane: initial_pane, surfaces, last_mouse_pane: Some(initial_pane), + mouse_capture_pane: None, last_mouse_point: None, last_mouse_physical_position: None, active_split_drag: None, modifiers: ModifiersState::empty(), + key_modifiers: input::KeyModifierState::default(), occluded: false, title: "Noa".to_string(), title_override: None, diff --git a/crates/noa-app/src/app/remote_ui.rs b/crates/noa-app/src/app/remote_ui.rs index 229d8bb8..14d940b7 100644 --- a/crates/noa-app/src/app/remote_ui.rs +++ b/crates/noa-app/src/app/remote_ui.rs @@ -1023,6 +1023,15 @@ impl App { } } + /// Characters in the endpoint input field (0 outside that phase), for + /// the IME candidate-window anchor. + pub(in crate::app) fn remote_ui_input_chars(&self) -> usize { + match self.remote_ui.as_ref().map(|session| &session.phase) { + Some(RemoteUiPhase::EndpointInput { buffer, .. }) => buffer.chars().count(), + _ => 0, + } + } + pub(in crate::app) fn push_remote_ui_text(&mut self, text: &str) { let filtered = text .chars() diff --git a/crates/noa-app/src/app/render.rs b/crates/noa-app/src/app/render.rs index 23b5ecbe..524c979c 100644 --- a/crates/noa-app/src/app/render.rs +++ b/crates/noa-app/src/app/render.rs @@ -459,6 +459,9 @@ impl App { // early-return below, so a background tab tracks its shell instead of // freezing at its last-foreground title (tab-close title-freeze fix). self.refresh_window_title(window_id); + // Redraws and IME events share the modal-aware anchor, including + // frame requests caused by preedit or a card layout change. + self.update_focused_ime_cursor_area(window_id); #[cfg(target_os = "macos")] let has_visible_background_image = self.background_image.has_visible_image(); let (Some(gpu), Some(state)) = (self.gpu.as_mut(), self.windows.get_mut(&window_id)) else { @@ -738,20 +741,6 @@ impl App { crate::macos_window::set_represented_url(&state.window, resolved.as_deref()); state.proxy_icon_cwd = new_cwd; } - if let Some((_, rect, snapshot)) = snapshots - .iter() - .find(|(pane_id, _, _)| *pane_id == state.focused_pane) - { - update_ime_cursor_area( - &state.window, - gpu.fonts.get(state.font_px).metrics(), - snapshot.cursor.x, - snapshot.cursor.y, - *rect, - self.padding, - ); - } - let panes = snapshots .iter() .map(|(pane_id, rect, snapshot)| PaneFrame { diff --git a/crates/noa-app/src/app/scratch_terminal.rs b/crates/noa-app/src/app/scratch_terminal.rs index f745f56f..693c00da 100644 --- a/crates/noa-app/src/app/scratch_terminal.rs +++ b/crates/noa-app/src/app/scratch_terminal.rs @@ -401,10 +401,12 @@ impl App { focused_pane: initial_pane, surfaces, last_mouse_pane: Some(initial_pane), + mouse_capture_pane: None, last_mouse_point: None, last_mouse_physical_position: None, active_split_drag: None, modifiers: ModifiersState::empty(), + key_modifiers: input::KeyModifierState::default(), occluded: false, title: "Scratch Terminal".to_string(), title_override: None, diff --git a/crates/noa-app/src/app/split_ops.rs b/crates/noa-app/src/app/split_ops.rs index c9152084..a823bff0 100644 --- a/crates/noa-app/src/app/split_ops.rs +++ b/crates/noa-app/src/app/split_ops.rs @@ -515,6 +515,9 @@ impl App { let Some(mut surface) = source_state.surfaces.remove(&pane) else { return false; }; + if source_state.mouse_capture_pane == Some(pane) { + source_state.mouse_capture_pane = None; + } source_state.split_tree = transform.source_tree; if source_state.zoomed == Some(pane) { source_state.zoomed = None; diff --git a/crates/noa-app/src/app/state.rs b/crates/noa-app/src/app/state.rs index 06c53caf..bb084d88 100644 --- a/crates/noa-app/src/app/state.rs +++ b/crates/noa-app/src/app/state.rs @@ -270,6 +270,10 @@ pub(super) struct WindowState { pub(super) focused_pane: PaneId, pub(super) surfaces: HashMap, pub(super) last_mouse_pane: Option, + /// The pane that received the live left-button press. Motion and the + /// matching release route to it even when the pointer has crossed into a + /// sibling split (xterm/Ghostty grab semantics); `None` outside a press. + pub(super) mouse_capture_pane: Option, pub(super) last_mouse_point: Option, /// Raw physical pointer position from the most recent `CursorMoved`. /// Kept alongside `last_mouse_point`/`last_mouse_pane` for handlers that @@ -280,6 +284,7 @@ pub(super) struct WindowState { pub(super) active_split_drag: Option, /// Modifier state tracked by winit for this native window's view. pub(super) modifiers: ModifiersState, + pub(super) key_modifiers: input::KeyModifierState, pub(super) occluded: bool, /// Whether this window was *created* with `with_transparent(true)`. /// AppKit fixes a window's opacity at creation — a window built opaque diff --git a/crates/noa-app/src/atomic_write.rs b/crates/noa-app/src/atomic_write.rs new file mode 100644 index 00000000..3b5214e2 --- /dev/null +++ b/crates/noa-app/src/atomic_write.rs @@ -0,0 +1,110 @@ +//! Crash- and race-safe whole-file replacement: write to a sibling staging +//! file, `sync_all`, then `rename` over the target, so a reader never sees a +//! truncated file and a crash mid-write leaves the previous good file. +//! +//! The staging name is unique per process *and* per call (`create_new`), so +//! two noa processes saving the same path at once can never share one +//! staging file — the shared `path.with_extension("tmp")` a store used to +//! pick let the loser's open descriptor rewrite the winner's already +//! published file and then fail its own rename, leaving "save failed" in the +//! UI with the failed content on disk (session store B03, favorites N05). +//! Which of two concurrent whole-file saves lands last is still unordered; +//! only corruption and phantom failures are ruled out here. + +use std::fs; +use std::io::{self, Write}; +use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicU64, Ordering}; + +/// Replace the file at `path` with `bytes`, creating the parent directory. +pub(crate) fn write_atomic(path: &Path, bytes: &[u8]) -> io::Result<()> { + if let Some(parent) = path.parent() { + fs::create_dir_all(parent)?; + } + let (tmp, mut file) = loop { + let tmp = staging_path(path); + match fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&tmp) + { + Ok(file) => break (tmp, file), + // A crashed process with a reused PID may have left this exact + // name behind; the counter makes the next candidate fresh. + Err(err) if err.kind() == io::ErrorKind::AlreadyExists => continue, + Err(err) => return Err(err), + } + }; + let result = (|| { + file.write_all(bytes)?; + file.sync_all()?; + fs::rename(&tmp, path) + })(); + drop(file); + if result.is_err() { + let _ = fs::remove_file(&tmp); + } + result +} + +/// `....tmp` beside `path`, unique per process and call. +fn staging_path(path: &Path) -> PathBuf { + static SEQ: AtomicU64 = AtomicU64::new(0); + let name = path + .file_name() + .map(|n| n.to_string_lossy().into_owned()) + .unwrap_or_else(|| "file".to_string()); + path.with_file_name(format!( + ".{name}.{}.{}.tmp", + std::process::id(), + SEQ.fetch_add(1, Ordering::Relaxed) + )) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn temp_dir(tag: &str) -> PathBuf { + let dir = std::env::temp_dir().join(format!( + "noa-atomic-write-{tag}-{}-{}", + std::process::id(), + SEQ_TEST.fetch_add(1, Ordering::Relaxed) + )); + fs::create_dir_all(&dir).unwrap(); + dir + } + static SEQ_TEST: AtomicU64 = AtomicU64::new(0); + + #[test] + fn staging_names_are_unique_per_call() { + let path = Path::new("/some/dir/favorites"); + let a = staging_path(path); + let b = staging_path(path); + assert_ne!(a, b); + assert_eq!(a.parent(), path.parent()); + assert!( + a.file_name() + .unwrap() + .to_string_lossy() + .starts_with(".favorites.") + ); + assert!(a.extension().is_some_and(|ext| ext == "tmp")); + } + + #[test] + fn write_replaces_the_file_and_leaves_no_staging_file_behind() { + let dir = temp_dir("replace"); + let path = dir.join("nested").join("store"); + write_atomic(&path, b"one").unwrap(); + write_atomic(&path, b"two").unwrap(); + assert_eq!(fs::read(&path).unwrap(), b"two"); + let leftovers: Vec<_> = fs::read_dir(path.parent().unwrap()) + .unwrap() + .map(|entry| entry.unwrap().file_name()) + .filter(|name| name != "store") + .collect(); + assert!(leftovers.is_empty(), "{leftovers:?}"); + fs::remove_dir_all(&dir).unwrap(); + } +} diff --git a/crates/noa-app/src/input.rs b/crates/noa-app/src/input.rs index 3578f4d7..82aa348d 100644 --- a/crates/noa-app/src/input.rs +++ b/crates/noa-app/src/input.rs @@ -14,7 +14,7 @@ mod text; pub(crate) use ime::ImeState; #[cfg(test)] use key::encode_key; -pub(crate) use key::{encode_enter_key, encode_key_with_modes}; +pub(crate) use key::{KeyModifierState, encode_enter_key, encode_key_with_modes}; pub use paste::encode_paste; pub(crate) use paste::{applescript_input_bytes, paste_is_unsafe, raw_input_bytes}; #[cfg(test)] diff --git a/crates/noa-app/src/input/key.rs b/crates/noa-app/src/input/key.rs index 142259a8..6cfe00f4 100644 --- a/crates/noa-app/src/input/key.rs +++ b/crates/noa-app/src/input/key.rs @@ -3,6 +3,35 @@ use winit::keyboard::{Key, KeyCode, ModifiersState, NamedKey, PhysicalKey}; use super::kitty::{KittyOutcome, encode_kitty}; use super::text::encode_text; +/// The Option/Alt classification belongs to a physical press, including its +/// repeats and release: winit's release events carry no composed text. +#[derive(Default)] +pub(crate) struct KeyModifierState { + alt_by_key: std::collections::HashMap, +} + +impl KeyModifierState { + pub(crate) fn alt_sends_esc( + &mut self, + key: PhysicalKey, + pressed: bool, + repeat: bool, + event_alt_sends_esc: bool, + ) -> bool { + if !pressed { + return self.alt_by_key.remove(&key).unwrap_or(event_alt_sends_esc); + } + if !repeat { + self.alt_by_key.insert(key, event_alt_sends_esc); + } + *self.alt_by_key.entry(key).or_insert(event_alt_sends_esc) + } + + pub(crate) fn clear(&mut self) { + self.alt_by_key.clear(); + } +} + /// Encode a pressed key into the bytes that should be written to the pty, if /// any. `app_cursor_keys` mirrors `ModeState::app_cursor_keys()` (DECCKM): /// when set, arrow keys send `SS3` (`ESC O `) instead of `CSI` @@ -47,8 +76,8 @@ pub fn encode_key( /// /// `alt_sends_esc` says whether Alt held with this press should ESC-prefix the /// produced text. On macOS the Option key composes characters unless -/// `macos-option-as-alt` claims it, so the caller decides per event; on other -/// platforms it is simply `true`. +/// `macos-option-as-alt` claims it, so the caller retains the press's verdict +/// through repeats and release; on other platforms it is simply `true`. #[allow(clippy::too_many_arguments)] pub fn encode_key_with_modes( logical_key: &Key, @@ -75,6 +104,7 @@ pub fn encode_key_with_modes( physical_key, text, mods, + alt_sends_esc, kitty_flags, pressed, repeat, diff --git a/crates/noa-app/src/input/kitty.rs b/crates/noa-app/src/input/kitty.rs index 32253ee6..1f2b11c9 100644 --- a/crates/noa-app/src/input/kitty.rs +++ b/crates/noa-app/src/input/kitty.rs @@ -44,6 +44,13 @@ fn kitty_modifier_value(mods: ModifiersState) -> u32 { value } +/// `alt_sends_esc` is the caller's press-paired verdict on whether a held Alt +/// is *Alt* or a macOS Option that composed `text` (see +/// `encode_key_with_modes`). A composing Option is not a modifier for the +/// "does this key escape-encode" decision (Ghostty's `effectiveMods`): the +/// character it produced is plain text under every flag set. The reported +/// modifier field still carries it (Ghostty reports `all_mods`), and it only +/// suppresses associated text when it is claimed as Alt. #[allow(clippy::too_many_arguments)] pub(super) fn encode_kitty( logical_key: &Key, @@ -51,6 +58,7 @@ pub(super) fn encode_kitty( physical_key: Option, text: Option<&str>, mods: ModifiersState, + alt_sends_esc: bool, flags: u8, pressed: bool, repeat: bool, @@ -74,81 +82,94 @@ pub(super) fn encode_kitty( }; let mods_value = kitty_modifier_value(mods); - let has_non_shift = mods.control_key() || mods.alt_key() || mods.super_key(); + let alt_is_modifier = mods.alt_key() && alt_sends_esc; + let has_non_shift = mods.control_key() || alt_is_modifier || mods.super_key(); + // Physical keypad keys carry dedicated code points: Ghostty's kitty + // table keys on the physical key, so KP_Enter is 57414, never Enter's + // 13. Text-producing keypad keys (digits, operators) stay legacy text + // without a non-shift modifier unless report-all forces the escape + // form, exactly like their main-block twins; the non-text ones + // (KP_Enter) always escape-encode under any flag, per the disambiguate + // rule for keypad keys (N11). + let keypad = keypad_key_code(physical_key).map(|number| KittyKey { + number, + suffix: b'u', + shifted: None, + }); // Classify the key and decide whether it escape-encodes under these flags. - let key = match logical_key { - Key::Named(NamedKey::Escape) => KittyKey { - number: 27, - suffix: b'u', - shifted: None, - }, - Key::Named(NamedKey::Enter) => { - if mods_value == 1 && !report_all { - return legacy_or_ignore(event); - } - KittyKey { - number: 13, - suffix: b'u', - shifted: None, - } - } - Key::Named(NamedKey::Tab) => { - if mods_value == 1 && !report_all { + let key = match (keypad, logical_key) { + (Some(key), Key::Character(_)) => { + if !report_all && !has_non_shift { return legacy_or_ignore(event); } - KittyKey { - number: 9, - suffix: b'u', - shifted: None, - } + key } - Key::Named(NamedKey::Backspace) => { - if mods_value == 1 && !report_all { - return legacy_or_ignore(event); - } - KittyKey { - number: 127, + (Some(key), _) => key, + (None, logical_key) => match logical_key { + Key::Named(NamedKey::Escape) => KittyKey { + number: 27, suffix: b'u', shifted: None, + }, + Key::Named(NamedKey::Enter) => { + if mods_value == 1 && !report_all { + return legacy_or_ignore(event); + } + KittyKey { + number: 13, + suffix: b'u', + shifted: None, + } } - } - Key::Named(NamedKey::Space) => { - if !report_all && !has_non_shift { - return legacy_or_ignore(event); - } - KittyKey { - number: 32, - suffix: b'u', - shifted: None, + Key::Named(NamedKey::Tab) => { + if mods_value == 1 && !report_all { + return legacy_or_ignore(event); + } + KittyKey { + number: 9, + suffix: b'u', + shifted: None, + } } - } - Key::Named(named) => match functional_key(*named) { - // Functional keys (arrows, F-keys, Home/End/...) always escape-encode. - Some((number, suffix)) => KittyKey { - number, - suffix, - shifted: None, - }, - // Modifier keys alone are reported only with report-all-keys. - None => match modifier_key_code(physical_key) { - Some(number) if report_all => KittyKey { - number, + Key::Named(NamedKey::Backspace) => { + if mods_value == 1 && !report_all { + return legacy_or_ignore(event); + } + KittyKey { + number: 127, suffix: b'u', shifted: None, - }, - _ => return KittyOutcome::Ignore, - }, - }, - Key::Character(s) => { - // Numpad keys get their dedicated codes only under report-all. - if report_all && let Some(number) = keypad_key_code(physical_key) { + } + } + Key::Named(NamedKey::Space) => { + if !report_all && !has_non_shift { + return legacy_or_ignore(event); + } KittyKey { - number, + number: 32, suffix: b'u', shifted: None, } - } else { + } + Key::Named(named) => match functional_key(*named) { + // Functional keys (arrows, F-keys, Home/End/...) always escape-encode. + Some((number, suffix)) => KittyKey { + number, + suffix, + shifted: None, + }, + // Modifier keys alone are reported only with report-all-keys. + None => match modifier_key_code(physical_key) { + Some(number) if report_all => KittyKey { + number, + suffix: b'u', + shifted: None, + }, + _ => return KittyOutcome::Ignore, + }, + }, + Key::Character(s) => { let Some((base, shifted)) = character_key_codes(s, unmodified_key, mods) else { return legacy_or_ignore(event); }; @@ -163,12 +184,13 @@ pub(super) fn encode_kitty( shifted, } } - } - _ => return legacy_or_ignore(event), + _ => return legacy_or_ignore(event), + }, }; // Associated text: only for press/repeat, only when no modifier other than - // shift is active, and only for genuinely printable text. + // shift (or a composing Option) is active, and only for genuinely + // printable text. let assoc_text = if report_text && event != 3 && !has_non_shift { associated_text_codepoints(text) } else { diff --git a/crates/noa-app/src/input/tests.rs b/crates/noa-app/src/input/tests.rs index e172c319..540cd69a 100644 --- a/crates/noa-app/src/input/tests.rs +++ b/crates/noa-app/src/input/tests.rs @@ -1574,3 +1574,171 @@ fn kitty_flags_take_precedence_over_modify_other_keys() { ); assert_eq!(bytes, Some(b"\x1b[105;5u".to_vec())); } + +// N09: a macOS Option that composed the delivered character (`alt_sends_esc` +// false) is not Alt for the Kitty escape decision — Ghostty's +// `effectiveMods` strips a consumed modifier — so the composed text passes +// through as text under disambiguate, and under report-all the sequence +// keeps the modifier bit but still carries the associated text. +#[test] +fn kitty_composing_option_is_not_alt() { + let press = |flags: u8, alt_sends_esc: bool| { + encode_key_with_modes( + &Key::Character("å".into()), + Some(&Key::Character("a".into())), + None, + Some("å"), + ModifiersState::ALT, + alt_sends_esc, + false, + false, + flags, + false, + true, + false, + ) + }; + assert_eq!( + press(KITTY_DISAMBIGUATE, false), + Some("å".as_bytes().to_vec()) + ); + // Option claimed as Alt (`macos-option-as-alt`): a real Alt+a chord. + assert_eq!( + press(KITTY_DISAMBIGUATE, true), + Some(b"\x1b[97;3u".to_vec()) + ); + assert_eq!( + press(KITTY_REPORT_ALL_KEYS | KITTY_REPORT_ASSOCIATED_TEXT, false), + Some(b"\x1b[97;3;229u".to_vec()) + ); + assert_eq!( + press(KITTY_REPORT_ALL_KEYS | KITTY_REPORT_ASSOCIATED_TEXT, true), + Some(b"\x1b[97;3u".to_vec()) + ); + // winit omits text on release; it must retain the press's composition + // classification so a legacy text press has no unpaired Kitty release. + assert_eq!( + encode_key_with_modes( + &Key::Character("å".into()), + Some(&Key::Character("a".into())), + Some(PhysicalKey::Code(KeyCode::KeyA)), + None, + ModifiersState::ALT, + false, + false, + false, + KITTY_DISAMBIGUATE | KITTY_REPORT_EVENT_TYPES, + false, + false, + false, + ), + None + ); +} + +#[test] +fn option_classification_survives_repeat_and_release_per_physical_key() { + let mut state = KeyModifierState::default(); + let composed = PhysicalKey::Code(KeyCode::KeyA); + let alt = PhysicalKey::Code(KeyCode::KeyB); + assert!(!state.alt_sends_esc(composed, true, false, false)); + assert!(state.alt_sends_esc(alt, true, false, true)); + // Missing/different repeat text must not reclassify the held key. + assert!(!state.alt_sends_esc(composed, true, true, true)); + let flags = KITTY_DISAMBIGUATE | KITTY_REPORT_EVENT_TYPES; + let release = |state: &mut KeyModifierState, physical, flags| { + encode_key_with_modes( + &Key::Character("a".into()), + None, + Some(physical), + None, + ModifiersState::ALT, + state.alt_sends_esc(physical, false, false, true), + false, + false, + flags, + false, + false, + false, + ) + }; + assert_eq!(release(&mut state, composed, flags), None); + assert_eq!( + release(&mut state, alt, flags), + Some(b"\x1b[97;3:3u".to_vec()) + ); + // Report-all still pairs its encoded press with an encoded release. + assert!(!state.alt_sends_esc(composed, true, false, false)); + assert_eq!( + release(&mut state, composed, flags | KITTY_REPORT_ALL_KEYS), + Some(b"\x1b[97;3:3u".to_vec()) + ); + assert!(!state.alt_sends_esc(composed, true, false, false)); + state.clear(); + assert!(state.alt_sends_esc(composed, false, false, true)); + // Reusing the same key starts a fresh classification. + assert!(state.alt_sends_esc(composed, true, false, true)); +} + +// N11: the keypad Enter is a distinct Kitty key (57414), reachable even +// though winit reports its *logical* key as plain `Enter`. +#[test] +fn kitty_keypad_enter_uses_its_dedicated_code() { + let press = |flags: u8, mods: ModifiersState| { + encode_key_with_modes( + &Key::Named(NamedKey::Enter), + None, + Some(PhysicalKey::Code(KeyCode::NumpadEnter)), + Some("\r"), + mods, + true, + false, + false, + flags, + false, + true, + false, + ) + }; + assert_eq!( + press(KITTY_REPORT_ALL_KEYS, ModifiersState::empty()), + Some(b"\x1b[57414u".to_vec()) + ); + // Non-text keypad keys escape-encode under disambiguate too (Ghostty + // keys its table on the physical key, so KP_Enter never takes Enter's + // bare-CR exemption). + assert_eq!( + press(KITTY_DISAMBIGUATE, ModifiersState::empty()), + Some(b"\x1b[57414u".to_vec()) + ); + assert_eq!( + press(KITTY_DISAMBIGUATE, ModifiersState::CONTROL), + Some(b"\x1b[57414;5u".to_vec()) + ); + // A bare keypad digit is still text under disambiguate; a modified one + // reports its keypad code, not the main-block digit's. + let digit = |flags: u8, mods: ModifiersState| { + encode_key_with_modes( + &Key::Character("5".into()), + None, + Some(PhysicalKey::Code(KeyCode::Numpad5)), + Some("5"), + mods, + true, + false, + false, + flags, + false, + true, + false, + ) + }; + assert_eq!( + digit(KITTY_DISAMBIGUATE, ModifiersState::empty()), + Some(b"5".to_vec()) + ); + assert_eq!( + digit(KITTY_DISAMBIGUATE, ModifiersState::CONTROL), + Some(b"\x1b[57404;5u".to_vec()) + ); +} diff --git a/crates/noa-app/src/lib.rs b/crates/noa-app/src/lib.rs index 7edcea31..c194989c 100644 --- a/crates/noa-app/src/lib.rs +++ b/crates/noa-app/src/lib.rs @@ -6,6 +6,7 @@ mod anim; mod app; mod app_actions; +mod atomic_write; pub mod auto_approve; mod branch_poll; mod chrome; diff --git a/crates/noa-app/src/macos_overlay.rs b/crates/noa-app/src/macos_overlay.rs index 226c039d..09039e49 100644 --- a/crates/noa-app/src/macos_overlay.rs +++ b/crates/noa-app/src/macos_overlay.rs @@ -25,7 +25,10 @@ mod tests; #[cfg(target_os = "macos")] pub(crate) use model::cg; -pub(crate) use model::{NativeOverlayCache, OverlayColors, PaneRectPt, TITLE_PROMPT_HINT}; +pub(crate) use model::{ + CaretPt, NativeOverlayCache, OverlayColors, PaneRectPt, TITLE_PROMPT_HINT, palette_query_caret, + theme_settings_caret, title_prompt_caret, +}; pub(crate) use sync::{ sync_command_palette, sync_confirm_dialog, sync_process_monitor, sync_scratch_badge, sync_theme_settings, sync_title_prompt, sync_toast, diff --git a/crates/noa-app/src/macos_overlay/imp/appkit.rs b/crates/noa-app/src/macos_overlay/imp/appkit.rs index af3b4425..ee36e602 100644 --- a/crates/noa-app/src/macos_overlay/imp/appkit.rs +++ b/crates/noa-app/src/macos_overlay/imp/appkit.rs @@ -1,6 +1,7 @@ use crate::macos_overlay::model::{ - OverlayColors, PaneRectPt, ProcessMonitorViewModel, ThemeSettingsViewModel, Tone, - overlay_scroll_window, + OverlayColors, PaneRectPt, ProcessMonitorViewModel, SETTINGS_DESCRIPTION_H, SETTINGS_FOOTER_H, + SETTINGS_ROW_H, SETTINGS_SEARCH_H, SETTINGS_TOP, THEME_FILTER_TOP, THEME_LIST_TOP, THEME_ROW_H, + ThemeSettingsLayout, ThemeSettingsViewModel, Tone, overlay_scroll_window, }; use crate::theme_settings::{Liveness, ThemeSettingsMode}; use noa_render::{CommandPaletteSnapshot, ConfirmDialogSnapshot, PaletteRow}; @@ -34,17 +35,13 @@ const ID_TITLE_PROMPT: &str = "noa.native-overlay.title-prompt"; const ID_TOAST: &str = "noa.native-overlay.toast"; const ID_SCRATCH_BADGE: &str = "noa.native-overlay.scratch-badge"; -/// Palette metrics (points). -const PALETTE_WIDTH: f64 = 560.0; -const QUERY_ROW_H: f64 = 44.0; -const ENTRY_ROW_H: f64 = 26.0; -const HEADER_ROW_H: f64 = 24.0; -const LIST_PAD_V: f64 = 6.0; -const CARD_PAD_H: f64 = 16.0; +/// Palette metrics (points). The widths/row heights the IME caret anchor +/// also needs live in `model.rs` so the two can't drift. +use crate::macos_overlay::model::{ + CARD_PAD_H, ENTRY_ROW_H, HEADER_ROW_H, LIST_PAD_V, PaletteCardLayout, QUERY_ROW_H, + THEME_SETTINGS_WIDTH, TITLE_PROMPT_H, TITLE_PROMPT_WIDTH, +}; const CARD_RADIUS: f64 = 12.0; -/// Max list rows (headers + entries) visible at once — matches the wgpu -/// card's 12-row window. -const PALETTE_CAPACITY: usize = 12; const SCRIM_ALPHA: f64 = 0.25; /// Balance a `+1` (alloc/init) object that a superview now retains: @@ -628,32 +625,18 @@ pub(in crate::macos_overlay) fn rebuild_palette( return; }; - // Window capacity bounded by the pane height so the list never - // runs past the card's bottom edge on a short pane. - let capacity = (((pane.h - 24.0 - QUERY_ROW_H - 1.0 - LIST_PAD_V * 2.0) / ENTRY_ROW_H) - as usize) - .clamp(3, PALETTE_CAPACITY); - let (offset, shown) = overlay_scroll_window(snap.rows.len(), snap.selected, capacity); + let PaletteCardLayout { + card_w, + card_h, + card_x, + card_top, + offset, + shown, + } = PaletteCardLayout::new(pane, snap); let visible = &snap.rows[offset..offset + shown]; let empty = snap.rows.is_empty(); - let list_h: f64 = if empty { - 36.0 - } else { - visible - .iter() - .map(|row| match row { - PaletteRow::Header { .. } => HEADER_ROW_H, - PaletteRow::Entry { .. } => ENTRY_ROW_H, - }) - .sum::() - + LIST_PAD_V * 2.0 - }; - let card_w = PALETTE_WIDTH.min(pane.w - 32.0).max(280.0); - let card_h = (QUERY_ROW_H + 1.0 + list_h).min(pane.h - 24.0); - let card_x = (pane.w - card_w) / 2.0; - let card_y_top = (pane.h * 0.14).min(pane.h - card_h).max(8.0); let card_frame = NSRect::new( - NSPoint::new(card_x, from_top(pane.h, card_y_top, card_h)), + NSPoint::new(card_x, from_top(pane.h, card_top, card_h)), NSSize::new(card_w, card_h), ); let (root, effect) = card_for( @@ -863,7 +846,7 @@ pub(in crate::macos_overlay) fn rebuild_theme_settings( return; }; - let card_w = 660.0_f64.min(pane.w - 32.0).max(320.0); + let card_w = THEME_SETTINGS_WIDTH.min(pane.w - 32.0).max(320.0); // The card height is content-driven: title block + exactly one // section (the theme list+sample pane in Theme mode, the settings // rows in Settings mode — a session never shows both, DEC-2) + @@ -878,46 +861,31 @@ pub(in crate::macos_overlay) fn rebuild_theme_settings( // Question, resolved differently per path since one has a dynamic // card height and the other doesn't). Settings mode is unaffected // — it uses its own independent `settings_top`, not this constant. - let list_top = 106.0; + let list_top = THEME_LIST_TOP; let chip_row_y = 84.0; - let row_h = 24.0; - let srow_h = 23.0; - let settings_header_h = 20.0; - let footer_h = 34.0; - let avail = (pane.h - 24.0).max(240.0); + let row_h = THEME_ROW_H; + let srow_h = SETTINGS_ROW_H; + let footer_h = SETTINGS_FOOTER_H; let settings_total = vm.settings_visible.len(); // Settings mode has no filter line/theme list above it, so its // section header sits where the "Theme"/"Sample" column headers // otherwise would (y=46); rows start directly below the header. - let settings_top = 46.0 + settings_header_h; + let settings_top = SETTINGS_TOP; // R-6/R-5 fixed lines (Addendum D-3/FM-04): always reserve the // description line; reserve the search line only while active. - let description_h = 19.0; - let search_h = 16.0; - - let (list_rows, settings_rows, card_h) = match vm.mode { - ThemeSettingsMode::Theme => { - let needed = |list_rows: usize| list_top + list_rows as f64 * row_h + footer_h; - let mut list_rows = vm.themes.len().max(1); - while needed(list_rows) > avail && list_rows > 3 { - list_rows -= 1; - } - (list_rows, 0usize, needed(list_rows).min(avail)) - } - ThemeSettingsMode::Settings => { - let (settings_rows, height) = crate::macos_overlay::model::settings_rows_budget( - settings_total, - avail, - settings_top, - srow_h, - footer_h, - description_h, - search_h, - vm.search_active, - ); - (0usize, settings_rows, height) - } - }; + let description_h = SETTINGS_DESCRIPTION_H; + let search_h = SETTINGS_SEARCH_H; + let ThemeSettingsLayout { + list_rows, + settings_rows, + card_h, + } = ThemeSettingsLayout::new( + pane.h, + vm.mode, + vm.themes.len(), + settings_total, + vm.search_active, + ); let card_frame = NSRect::new( NSPoint::new( (pane.w - card_w) / 2.0, @@ -1017,7 +985,7 @@ pub(in crate::macos_overlay) fn rebuild_theme_settings( mono_digit_font(12.0), muted, NSRect::new( - NSPoint::new(pad, from_top(card_h, 64.0, 16.0)), + NSPoint::new(pad, from_top(card_h, THEME_FILTER_TOP, SETTINGS_SEARCH_H)), NSSize::new(col_split - pad - 8.0 - count_w, 16.0), ), ); @@ -1843,8 +1811,8 @@ pub(in crate::macos_overlay) fn rebuild_title_prompt( return; }; - let card_w = 420.0_f64.min(pane.w - 32.0).max(240.0); - let card_h = 104.0; + let card_w = TITLE_PROMPT_WIDTH.min(pane.w - 32.0).max(240.0); + let card_h = TITLE_PROMPT_H; let card_frame = NSRect::new( NSPoint::new( (pane.w - card_w) / 2.0, diff --git a/crates/noa-app/src/macos_overlay/model.rs b/crates/noa-app/src/macos_overlay/model.rs index 005cf6e5..c940c6eb 100644 --- a/crates/noa-app/src/macos_overlay/model.rs +++ b/crates/noa-app/src/macos_overlay/model.rs @@ -1,6 +1,6 @@ use std::hash::{Hash, Hasher}; -use noa_render::OverlayStyle; +use noa_render::{CommandPaletteSnapshot, OverlayStyle, PaletteRow}; /// Opaque `CGColorRef` for `msg_send!` returns/arguments. The `-CGColor` /// property returns `^{CGColor=}`, not an object (`@`) — typing it as @@ -69,6 +69,143 @@ impl NativeOverlayCache { /// card in `app.rs`. pub(crate) const TITLE_PROMPT_HINT: &str = "Enter to set \u{b7} Empty clears \u{b7} Esc to cancel"; +/// Card metrics (points) shared by the AppKit card builders (`imp/appkit.rs`) +/// and the IME caret anchors below, so the candidate window and the drawn +/// input row can't drift apart. +pub(crate) const PALETTE_WIDTH: f64 = 560.0; +pub(crate) const QUERY_ROW_H: f64 = 44.0; +pub(crate) const CARD_PAD_H: f64 = 16.0; +pub(crate) const ENTRY_ROW_H: f64 = 26.0; +pub(crate) const HEADER_ROW_H: f64 = 24.0; +pub(crate) const LIST_PAD_V: f64 = 6.0; +/// Maximum visible list rows, including headers. +const PALETTE_CAPACITY: usize = 12; +pub(crate) const TITLE_PROMPT_WIDTH: f64 = 420.0; +pub(crate) const TITLE_PROMPT_H: f64 = 104.0; +pub(crate) const THEME_SETTINGS_WIDTH: f64 = 660.0; +/// Mean advance of the 15pt system font the input rows use — an estimate +/// (the labels are proportional), good enough to put the candidate window +/// at the end of the typed text rather than at its start. +const INPUT_FONT_ADVANCE: f64 = 8.3; +const INPUT_ROW_H: f64 = 20.0; + +/// A caret rectangle in points, relative to the pane rect's origin (the +/// cards are laid out inside the focused pane's frame). +#[derive(Clone, Copy, Debug, PartialEq)] +pub(crate) struct CaretPt { + pub(crate) x: f64, + pub(crate) y: f64, + pub(crate) w: f64, + pub(crate) h: f64, +} + +/// Card placement and visible rows shared by native drawing and IME. +pub(crate) struct PaletteCardLayout { + pub(crate) card_w: f64, + pub(crate) card_h: f64, + pub(crate) card_x: f64, + pub(crate) card_top: f64, + pub(crate) offset: usize, + pub(crate) shown: usize, +} + +impl PaletteCardLayout { + pub(crate) fn new(pane: PaneRectPt, snapshot: &CommandPaletteSnapshot) -> Self { + let capacity = (((pane.h - 24.0 - QUERY_ROW_H - 1.0 - LIST_PAD_V * 2.0) / ENTRY_ROW_H) + as usize) + .clamp(3, PALETTE_CAPACITY); + let (offset, shown) = + overlay_scroll_window(snapshot.rows.len(), snapshot.selected, capacity); + let visible = &snapshot.rows[offset..offset + shown]; + let list_h = if visible.is_empty() { + 36.0 + } else { + visible + .iter() + .map(|row| match row { + PaletteRow::Header { .. } => HEADER_ROW_H, + PaletteRow::Entry { .. } => ENTRY_ROW_H, + }) + .sum::() + + LIST_PAD_V * 2.0 + }; + let card_w = PALETTE_WIDTH.min(pane.w - 32.0).max(280.0); + let card_h = (QUERY_ROW_H + 1.0 + list_h).min(pane.h - 24.0); + Self { + card_w, + card_h, + card_x: (pane.w - card_w) / 2.0, + card_top: (pane.h * 0.14).min(pane.h - card_h).max(8.0), + offset, + shown, + } + } +} + +/// The palette query row's caret after `query_chars` characters, including +/// the remote endpoint field, which uses the same card. +pub(crate) fn palette_query_caret( + pane: PaneRectPt, + snapshot: &CommandPaletteSnapshot, + query_chars: usize, +) -> CaretPt { + 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, + w: 1.0, + h: INPUT_ROW_H, + } +} + +/// The "Set Tab Title" prompt's caret: its input row is centred, so the +/// caret sits half the text's width right of the card's centre line. +pub(crate) fn title_prompt_caret(pane: PaneRectPt, input_chars: usize) -> CaretPt { + let card_w = TITLE_PROMPT_WIDTH.min(pane.w - 32.0).max(240.0); + let card_x = (pane.w - card_w) / 2.0; + let card_top = (pane.h * 0.30).min(pane.h - TITLE_PROMPT_H); + CaretPt { + x: card_x + card_w / 2.0 + input_chars as f64 * INPUT_FONT_ADVANCE / 2.0, + y: card_top + 40.0, + w: 1.0, + h: INPUT_ROW_H, + } +} + +/// The search/filter field uses the same content-driven card height as the +/// native builder, including filtered/empty lists and the Settings mode. +pub(crate) fn theme_settings_caret( + pane: PaneRectPt, + state: &crate::theme_settings::ThemeSettings, +) -> CaretPt { + use crate::theme_settings::{SettingsRowKind, ThemeSettingsMode}; + let card_w = THEME_SETTINGS_WIDTH.min(pane.w - 32.0).max(320.0); + let settings_total = if state.settings_search_active() { + state.settings_filtered_len() + } else { + SettingsRowKind::COUNT + }; + let layout = ThemeSettingsLayout::new( + pane.h, + state.mode(), + state.filtered_len().min(THEME_LIST_ROWS), + settings_total, + state.settings_search_active(), + ); + let card_x = (pane.w - card_w) / 2.0; + let card_top = (pane.h - layout.card_h) / 2.0; + 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, + w: 1.0, + h: SETTINGS_SEARCH_H, + } +} + /// A pane rectangle in AppKit points, top-left origin relative to the /// window's content view (i.e. physical px / scale factor). #[derive(Clone, Copy, Debug, PartialEq)] @@ -291,6 +428,66 @@ pub(crate) struct ThemeSettingsViewModel { /// Rows the theme list shows at once in the native card. const THEME_LIST_ROWS: usize = 8; +pub(crate) const THEME_FILTER_TOP: f64 = 64.0; +pub(crate) const THEME_LIST_TOP: f64 = 106.0; +pub(crate) const THEME_ROW_H: f64 = 24.0; +pub(crate) const SETTINGS_TOP: f64 = 66.0; +pub(crate) const SETTINGS_ROW_H: f64 = 23.0; +pub(crate) const SETTINGS_FOOTER_H: f64 = 34.0; +pub(crate) const SETTINGS_DESCRIPTION_H: f64 = 19.0; +pub(crate) const SETTINGS_SEARCH_H: f64 = 16.0; + +pub(crate) struct ThemeSettingsLayout { + pub(crate) list_rows: usize, + pub(crate) settings_rows: usize, + pub(crate) card_h: f64, +} + +impl ThemeSettingsLayout { + pub(crate) fn new( + pane_h: f64, + mode: crate::theme_settings::ThemeSettingsMode, + theme_rows: usize, + settings_total: usize, + search_active: bool, + ) -> Self { + use crate::theme_settings::ThemeSettingsMode; + let avail = (pane_h - 24.0).max(240.0); + match mode { + ThemeSettingsMode::Theme => { + let needed = + |rows: usize| THEME_LIST_TOP + rows as f64 * THEME_ROW_H + SETTINGS_FOOTER_H; + let mut list_rows = theme_rows.max(1); + while needed(list_rows) > avail && list_rows > 3 { + list_rows -= 1; + } + Self { + list_rows, + settings_rows: 0, + card_h: needed(list_rows).min(avail), + } + } + ThemeSettingsMode::Settings => { + let (settings_rows, card_h) = settings_rows_budget( + settings_total, + avail, + SETTINGS_TOP, + SETTINGS_ROW_H, + SETTINGS_FOOTER_H, + SETTINGS_DESCRIPTION_H, + SETTINGS_SEARCH_H, + search_active, + ); + Self { + list_rows: 0, + settings_rows, + card_h, + } + } + } + } +} + pub(crate) fn theme_settings_view_model( state: &crate::theme_settings::ThemeSettings, ) -> ThemeSettingsViewModel { diff --git a/crates/noa-app/src/macos_overlay/tests.rs b/crates/noa-app/src/macos_overlay/tests.rs index ab6e80ac..6436bfeb 100644 --- a/crates/noa-app/src/macos_overlay/tests.rs +++ b/crates/noa-app/src/macos_overlay/tests.rs @@ -1,8 +1,27 @@ use super::model::{ - NativeOverlayCache, OverlayColors, PaneRectPt, overlay_scroll_window, theme_settings_view_model, + CARD_PAD_H, NativeOverlayCache, OverlayColors, PALETTE_WIDTH, PaneRectPt, THEME_SETTINGS_WIDTH, + overlay_scroll_window, palette_query_caret, theme_settings_caret, theme_settings_view_model, + title_prompt_caret, }; use super::sync::theme_settings_sync_decision; use crate::theme_settings::{Liveness, ThemeSettings, ThemeSettingsInit, ThemeSettingsMode}; +use noa_render::{CommandPaletteSnapshot, PaletteRow}; + +fn palette_snapshot(entries: usize) -> CommandPaletteSnapshot { + CommandPaletteSnapshot { + query: String::new(), + rows: (0..entries) + .map(|index| PaletteRow::Entry { + title: format!("Result {index}"), + hint: None, + match_positions: Vec::new(), + enabled: true, + }) + .collect(), + selected: 0, + total_entries: entries, + } +} fn settings_init() -> ThemeSettingsInit { ThemeSettingsInit { @@ -344,3 +363,117 @@ fn pane_rect_pt_scales_from_px() { assert_eq!(rect.w, 400.0); assert_eq!(rect.h, 300.0); } + +// N12: the modal caret anchors land inside their card's input row, not at +// the terminal cursor, and advance with the typed text. +#[test] +fn modal_caret_anchors_sit_in_the_input_row_and_follow_the_text() { + let pane = PaneRectPt { + x: 0.0, + y: 0.0, + w: 1200.0, + h: 800.0, + }; + let card_x = (1200.0 - PALETTE_WIDTH) / 2.0; + let snapshot = palette_snapshot(0); + let empty = palette_query_caret(pane, &snapshot, 0); + assert_eq!(empty.x, card_x + CARD_PAD_H + 22.0); + assert_eq!(empty.y, 800.0 * 0.14 + 13.0); + let typed = palette_query_caret(pane, &snapshot, 10); + assert!(typed.x > empty.x && typed.y == empty.y); + + let title = title_prompt_caret(pane, 0); + assert_eq!(title.x, 600.0); + assert_eq!(title.y, 800.0 * 0.30 + 40.0); + assert!(title_prompt_caret(pane, 4).x > title.x); + + let theme_state = ThemeSettings::open(ThemeSettingsInit { + mode: ThemeSettingsMode::Theme, + ..settings_init() + }); + let theme = theme_settings_caret(pane, &theme_state); + assert_eq!(theme.x, (1200.0 - THEME_SETTINGS_WIDTH) / 2.0 + 20.0); + assert_eq!(theme.y, 298.0); + + // A pane narrower than the card clamps the card to the pane's width + // minus its margins; the caret must stay inside it. + let narrow = PaneRectPt { + x: 0.0, + y: 0.0, + w: 400.0, + h: 300.0, + }; + let caret = palette_query_caret(narrow, &snapshot, 0); + assert!(caret.x >= 16.0 && caret.x < 400.0 - 16.0); + assert!(caret.y > 0.0 && caret.y < 300.0); +} + +#[test] +fn theme_caret_uses_filtered_rows_short_panes_and_settings_search_layout() { + let pane = PaneRectPt { + x: 0.0, + y: 0.0, + w: 1200.0, + h: 800.0, + }; + let mut theme = ThemeSettings::open(ThemeSettingsInit { + mode: ThemeSettingsMode::Theme, + ..settings_init() + }); + // Eight displayed rows give a 332pt card; a 300pt pane fits five rows + // in a 260pt card. Both anchors sit 64pt below the actual card top. + assert_eq!(theme_settings_caret(pane, &theme).y, 298.0); + assert_eq!( + theme_settings_caret(PaneRectPt { h: 300.0, ..pane }, &theme).y, + 84.0 + ); + theme.push_text("no-such-theme-999999999", std::time::Instant::now()); + assert_eq!(theme.filtered_len(), 0); + // Empty lists retain one row: 106 + 24 + 34 = 164pt. + assert_eq!(theme_settings_caret(pane, &theme).y, 382.0); + + let mut settings = ThemeSettings::open(settings_init()); + settings.toggle_settings_search(); + settings.push_text("no-such-setting-999999999", std::time::Instant::now()); + assert!(settings.settings_search_active()); + assert_eq!(settings.settings_filtered_len(), 0); + // Settings reserve the description and search lines, yielding 158pt; + // their search input starts at 66pt rather than the theme's 64pt. + assert_eq!(theme_settings_caret(pane, &settings).y, 387.0); +} + +#[test] +fn palette_caret_uses_twelve_result_rows_in_a_short_pane() { + let pane = PaneRectPt { + h: 400.0, + ..test_rect() + }; + // Twelve 26pt entries + 12pt list padding + 45pt query/rule = 369pt. + // The card starts at 400 - 369 = 31pt, and the query at 31 + 13 = 44pt. + assert_eq!(palette_query_caret(pane, &palette_snapshot(12), 10).y, 44.0); +} + +#[test] +fn palette_caret_tracks_visible_headers_selection_and_pane_height() { + let pane = PaneRectPt { + h: 400.0, + ..test_rect() + }; + let mut snapshot = palette_snapshot(20); + snapshot.rows[0] = PaletteRow::Header { + label: "Commands".to_string(), + }; + // One 24pt header and eleven 26pt entries make a 367pt card. + assert_eq!(palette_query_caret(pane, &snapshot, 0).y, 46.0); + snapshot.selected = 19; + // Scrolling the header out replaces it with a 26pt entry. + assert_eq!(palette_query_caret(pane, &snapshot, 0).y, 44.0); + assert_eq!(palette_query_caret(pane, &palette_snapshot(0), 0).y, 69.0); + + let short = PaneRectPt { h: 300.0, ..pane }; + // Eight entries fit (265pt card), placing the query at 35 + 13pt. + assert_eq!(palette_query_caret(short, &snapshot, 0).y, 48.0); + let tiny = PaneRectPt { h: 100.0, ..pane }; + // The card height is capped at pane.h - 24 even at the three-row floor. + assert!((palette_query_caret(tiny, &snapshot, 0).y - 27.0).abs() < 1e-9); +} diff --git a/crates/noa-app/src/session.rs b/crates/noa-app/src/session.rs index 57986b9d..030be018 100644 --- a/crates/noa-app/src/session.rs +++ b/crates/noa-app/src/session.rs @@ -468,60 +468,13 @@ fn parse_remote(value: &json::Value) -> Option { }) } -/// Atomically write the session to `path`, creating the parent directory. -/// Writes to a sibling temp file then renames, so a crash mid-write cannot -/// truncate an existing good session file. -/// -/// The temp name is unique per process and per call (`create_new`), so two -/// noa processes saving to the same path at once cannot share one staging -/// file and interleave their bytes into the published `session.json` -/// (B03, 2026-09 audit #2). Which of two concurrent whole-file saves lands -/// last is still unordered; only corruption is ruled out here. +/// Atomically write the session to `path`, creating the parent directory +/// (staging file + rename, unique per process and call — see +/// [`crate::atomic_write`], B03 2026-09 audit #2). Which of two concurrent +/// whole-file saves lands last is still unordered; only corruption is ruled +/// out here. pub fn save(path: &Path, state: &SessionState) -> std::io::Result<()> { - use std::io::Write; - - if let Some(parent) = path.parent() { - fs::create_dir_all(parent)?; - } - let (tmp, mut file) = loop { - let tmp = staging_path(path); - match fs::OpenOptions::new() - .write(true) - .create_new(true) - .open(&tmp) - { - Ok(file) => break (tmp, file), - // A crashed process with a reused PID may have left this exact - // name behind; the counter makes the next candidate fresh. - Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => continue, - Err(err) => return Err(err), - } - }; - let result = (|| { - file.write_all(serialize(state).as_bytes())?; - file.sync_all()?; - fs::rename(&tmp, path) - })(); - drop(file); - if result.is_err() { - let _ = fs::remove_file(&tmp); - } - result -} - -/// `....tmp` beside `path`, unique per process and call. -fn staging_path(path: &Path) -> std::path::PathBuf { - use std::sync::atomic::{AtomicU64, Ordering}; - static SEQ: AtomicU64 = AtomicU64::new(0); - let name = path - .file_name() - .map(|n| n.to_string_lossy().into_owned()) - .unwrap_or_else(|| "session.json".to_string()); - path.with_file_name(format!( - ".{name}.{}.{}.tmp", - std::process::id(), - SEQ.fetch_add(1, Ordering::Relaxed) - )) + crate::atomic_write::write_atomic(path, serialize(state).as_bytes()) } /// Load and parse the session at `path`, or `None` if it is absent, unreadable, diff --git a/crates/noa-app/src/theme_favorites.rs b/crates/noa-app/src/theme_favorites.rs index 6b197e4f..223274ce 100644 --- a/crates/noa-app/src/theme_favorites.rs +++ b/crates/noa-app/src/theme_favorites.rs @@ -52,15 +52,14 @@ pub(crate) fn load(path: &Path) -> HashSet { } } -/// Atomically write `favorites` to `path` (temp file + rename), creating -/// the parent directory if needed — mirrors `session.rs::save`'s pattern. +/// Atomically write `favorites` to `path`, creating the parent directory if +/// needed. Uses the shared per-process/per-call staging file +/// ([`crate::atomic_write`]) rather than a fixed `favorites.tmp`: with the +/// fixed name two noa processes toggling at once shared one staging file, +/// and the loser could overwrite the already-published file through its +/// open descriptor and then report a failed save (N05). pub(crate) fn save(path: &Path, favorites: &HashSet) -> io::Result<()> { - if let Some(parent) = path.parent() { - fs::create_dir_all(parent)?; - } - let tmp = path.with_extension("tmp"); - fs::write(&tmp, serialize(favorites))?; - fs::rename(&tmp, path) + crate::atomic_write::write_atomic(path, serialize(favorites).as_bytes()) } /// `App`'s handle on the favorites store: lazily loaded (best-effort, empty diff --git a/crates/noa-config/src/writer.rs b/crates/noa-config/src/writer.rs index 4af0e1cb..0ed1db1b 100644 --- a/crates/noa-config/src/writer.rs +++ b/crates/noa-config/src/writer.rs @@ -212,6 +212,7 @@ pub fn write_config_updates(path: &Path, updates: &[(String, String)]) -> io::Re }; let updated = apply_updates(&original, updates); + let updated = repair_font_family_primaries(path, updated, updates); let target = resolve_symlinks(path)?; let parent = target.parent().ok_or_else(|| { @@ -249,6 +250,76 @@ pub fn write_config_updates(path: &Path, updates: &[(String, String)]) -> io::Re write_result } +/// [`apply_updates`] only sees the primary file, but the parser splices +/// `config-file` includes in at their directive's position and accumulates +/// every `font-family*` line into one ordered stack. A family contributed by +/// an include that precedes the rewritten slot — or by an include standing +/// in for a slot the primary file never had — therefore still heads the +/// effective list after a save, and the panel's "primary font" change is +/// silently ineffective (N04). Re-parse the would-be file with includes +/// expanded and, when the requested family did not land in front, rewrite +/// the key as a list reset followed by the new primary and every surviving +/// fallback (in effective order, deduplicated), all trailing the last +/// include so nothing can shadow them. +fn repair_font_family_primaries( + path: &Path, + mut updated: String, + updates: &[(String, String)], +) -> String { + for (key, value) in updates { + if !is_font_family_key(key) || value.is_empty() { + continue; + } + let (overrides, _) = crate::parse_overrides(path, &updated); + let families = font_families_for_key(&overrides.font, key); + if families.first() == Some(value) { + continue; + } + // The reset replaces the value line `apply_updates` just placed (the + // key's last occurrence) and, when an include follows it, is appended + // again after that include — so the primary + fallbacks appended + // below start from an empty list in every reload order. + updated = apply_updates(&updated, &[(key.clone(), String::new())]); + let mut lines = vec![(key.clone(), value.clone())]; + lines.extend( + families + .iter() + .filter(|family| *family != value) + .map(|family| (key.clone(), family.clone())), + ); + updated = append_directives(&updated, &lines); + } + updated +} + +fn font_families_for_key<'a>(font: &'a crate::FontConfig, key: &str) -> &'a [String] { + match key { + "font-family-bold" => &font.families_bold, + "font-family-italic" => &font.families_italic, + "font-family-bold-italic" => &font.families_bold_italic, + _ => &font.families, + } +} + +/// Append `lines` as `key = value` directives at the end of `text`, on the +/// file's dominant line terminator (same rule as [`apply_updates`]). +fn append_directives(text: &str, lines: &[(String, String)]) -> String { + let existing = split_lines_preserving_terminators(text); + let terminator = dominant_terminator(&existing); + let mut output = text.to_string(); + if existing + .last() + .is_some_and(|(_, terminator)| terminator.is_empty()) + { + output.push_str(terminator); + } + for (key, value) in lines { + output.push_str(&format!("{key} = {value}")); + output.push_str(terminator); + } + output +} + /// Upper bound on symlink hops before `resolve_symlinks` reports a cycle /// (Linux's `MAXSYMLINKS` — `fs::canonicalize` would fail at the same depth). const MAX_SYMLINK_HOPS: usize = 40; @@ -490,6 +561,60 @@ theme = 3024 Day\r fs::remove_dir_all(&dir).unwrap(); } + // N04: a family that only an include provides must not stay primary + // after the panel saves a different one — with the include either + // standing in for a missing slot or preceding the primary file's own. + #[test] + fn font_saves_beat_families_contributed_by_includes() { + for (label, main) in [ + ("include-only", "config-file = fonts.conf\n"), + ( + "include-before-slot", + "config-file = fonts.conf\nfont-family = Menlo\n", + ), + ] { + let dir = unique_temp_dir(&format!("font-include-{label}")); + fs::create_dir_all(&dir).unwrap(); + let path = dir.join("config"); + fs::write( + dir.join("fonts.conf"), + "font-family = Monaco\nfont-family = Courier\n", + ) + .unwrap(); + fs::write(&path, main).unwrap(); + + for family in ["Fira Code", "Hack"] { + write_config_updates(&path, &[("font-family".into(), family.into())]).unwrap(); + let source = fs::read_to_string(&path).unwrap(); + let (overrides, diagnostics) = crate::parse_overrides(&path, &source); + assert!(diagnostics.is_empty(), "{label}: {diagnostics:?}"); + assert_eq!( + overrides.font.families.first().map(String::as_str), + Some(family), + "{label}: the saved family heads the effective list" + ); + assert!( + overrides.font.families.contains(&"Monaco".to_string()) + && overrides.font.families.contains(&"Courier".to_string()), + "{label}: the include's families survive as fallbacks: {:?}", + overrides.font.families + ); + assert_eq!( + overrides.font.families.len(), + 3, + "{label}: no duplicate or stale entries: {:?}", + overrides.font.families + ); + } + // Untouched by the writer: the include file itself. + assert_eq!( + fs::read_to_string(dir.join("fonts.conf")).unwrap(), + "font-family = Monaco\nfont-family = Courier\n" + ); + fs::remove_dir_all(&dir).unwrap(); + } + } + #[test] fn font_family_reset_clears_trailing_includes_after_reload() { let dir = unique_temp_dir("font-reset-include"); diff --git a/crates/noa-grid/src/screen/edit.rs b/crates/noa-grid/src/screen/edit.rs index 64ed19b5..9b4f5ce9 100644 --- a/crates/noa-grid/src/screen/edit.rs +++ b/crates/noa-grid/src/screen/edit.rs @@ -5,13 +5,36 @@ use super::*; impl Screen { pub(crate) fn clear_scrollback(&mut self) { - if self.scrollback.len() > 0 { + self.collapse_scrollback(); + self.clear_selection(); + self.clear_search(); + } + + /// Drop the scrollback and collapse the session-absolute coordinate + /// space by its length: Kitty placements anchored in the cleared history + /// go, survivors in the live area re-anchor into the shrunken space so + /// they keep their screen position. Shared by [`Self::clear_scrollback`] + /// (the app's "Clear Scrollback" action) and `ED 3`, which used to be two + /// diverging copies — the former left every anchor `old_len` rows too + /// high and pushed visible images off-screen (N06). Returns the number + /// of rows removed, for callers that also shift a selection. + fn collapse_scrollback(&mut self) -> usize { + let old_len = self.scrollback.len(); + let old_live_top = self.live_area_abs_top(); + if old_len > 0 { self.invalidate_coordinate_space(); } self.scrollback.clear(); self.viewport_offset = 0; - self.clear_selection(); - self.clear_search(); + self.kitty_placements.retain_mut(|p| { + if p.anchor_abs_row < old_live_top { + false + } else { + p.anchor_abs_row -= old_len; + true + } + }); + old_len } /// Ghostty parity: the non-prompt branch of `Termio.clearScreen` — erase @@ -203,7 +226,9 @@ impl Screen { /// Index (IND / LF without CR): down one row, scrolling at the region bottom. pub fn index(&mut self) { self.cursor.pending_wrap = false; - if self.cursor.y == self.region.bottom { + if self.cursor.y == self.region.bottom + && (self.left_margin()..=self.right_margin()).contains(&self.cursor.x) + { self.scroll_up_region(1); } else if self.cursor.y + 1 < self.rows { self.cursor.y += 1; @@ -213,7 +238,9 @@ impl Screen { /// Reverse index (RI): up one row, scrolling down at the region top. pub fn reverse_index(&mut self) { self.cursor.pending_wrap = false; - if self.cursor.y == self.region.top { + if self.cursor.y == self.region.top + && (self.left_margin()..=self.right_margin()).contains(&self.cursor.x) + { self.scroll_down_region(1); } else if self.cursor.y > 0 { self.cursor.y -= 1; @@ -476,14 +503,32 @@ impl Screen { // ── horizontal / absolute motion ──────────────────────────────── + // A relative move stops at the margin in its direction of travel, + // unless the cursor is already beyond that margin. Absolute placement + // can put it there with DECOM off; then the screen edge is the limit. + fn cursor_left_bound(&self) -> u16 { + if self.cursor.x >= self.left_margin() { + self.left_margin() + } else { + 0 + } + } + + fn cursor_right_bound(&self) -> u16 { + if self.cursor.x <= self.right_margin() { + self.right_margin() + } else { + self.cols.saturating_sub(1) + } + } + pub fn carriage_return(&mut self) { - self.cursor.x = self.left_margin(); + self.cursor.x = self.cursor_left_bound(); self.cursor.pending_wrap = false; } pub fn backspace(&mut self) { - self.cursor.pending_wrap = false; - self.cursor.x = self.cursor.x.saturating_sub(1).max(self.left_margin()); + self.cursor_backward(1); } pub fn cursor_up(&mut self, n: u16) { @@ -511,19 +556,32 @@ impl Screen { pub fn cursor_forward(&mut self, n: u16) { self.cursor.pending_wrap = false; let n = n.max(1); - self.cursor.x = self.cursor.x.saturating_add(n).min(self.right_margin()); + self.cursor.x = self + .cursor + .x + .saturating_add(n) + .min(self.cursor_right_bound()); } pub fn cursor_backward(&mut self, n: u16) { self.cursor.pending_wrap = false; let n = n.max(1); - self.cursor.x = self.cursor.x.saturating_sub(n).max(self.left_margin()); + self.cursor.x = self + .cursor + .x + .saturating_sub(n) + .max(self.cursor_left_bound()); } + /// Absolute cursor placement (`CUP`/`HVP` with DECOM off). Ghostty's + /// `setCursorPos` bounds the target by the *screen* unless origin mode is + /// on — the left/right margins only constrain the origin-relative form + /// ([`Self::cursor_position_with_origin`]) — so a TUI that draws outside + /// a DECSLRM window lands where it asked, not snapped to the margin (N07). pub fn cursor_position(&mut self, row: u16, col: u16) { self.cursor.pending_wrap = false; self.cursor.y = row.saturating_sub(1).min(self.rows.saturating_sub(1)); - self.cursor.x = self.clamp_x_to_margins(col.saturating_sub(1)); + self.cursor.x = col.saturating_sub(1).min(self.cols.saturating_sub(1)); } pub(crate) fn cursor_position_with_origin(&mut self, row: u16, col: u16, origin: bool) { @@ -543,9 +601,10 @@ impl Screen { .min(self.right_margin()); } + /// `CHA`/`HPA` with DECOM off: screen-bounded like [`Self::cursor_position`]. pub fn cursor_col_abs(&mut self, col: u16) { self.cursor.pending_wrap = false; - self.cursor.x = self.clamp_x_to_margins(col.saturating_sub(1)); + self.cursor.x = col.saturating_sub(1).min(self.cols.saturating_sub(1)); } pub(crate) fn cursor_col_abs_with_origin(&mut self, col: u16, origin: bool) { @@ -578,10 +637,12 @@ impl Screen { .min(self.region.bottom); } + /// Home is the top-left of the scrolling region under DECOM and of the + /// screen otherwise (DECSLRM/DECSTBM/DECOM all home through here). pub(crate) fn home_cursor(&mut self, origin: bool) { self.cursor.pending_wrap = false; self.cursor.y = if origin { self.region.top } else { 0 }; - self.cursor.x = self.left_margin(); + self.cursor.x = if origin { self.left_margin() } else { 0 }; } pub(crate) fn reported_cursor_position(&self, origin: bool) -> (u16, u16) { @@ -599,14 +660,17 @@ impl Screen { self.cursor.x = self .tabstops .next(self.cursor.x, self.cols) - .min(self.right_margin()); + .min(self.cursor_right_bound()); } } pub fn tab_back(&mut self, n: u16) { self.cursor.pending_wrap = false; for _ in 0..n.max(1) { - self.cursor.x = self.tabstops.prev(self.cursor.x).max(self.left_margin()); + self.cursor.x = self + .tabstops + .prev(self.cursor.x) + .max(self.cursor_left_bound()); } } @@ -716,24 +780,7 @@ impl Screen { self.remove_placements_intersecting_grid_rows(0, last); } EraseDisplay::Scrollback => { - // Clearing scrollback collapses the absolute coordinate space by - // its length: drop placements anchored in the cleared history and - // re-anchor the survivors (live area) into the shrunken space. - let old_len = self.scrollback.len(); - let old_live_top = self.live_area_abs_top(); - if old_len > 0 { - self.invalidate_coordinate_space(); - } - self.scrollback.clear(); - self.viewport_offset = 0; - self.kitty_placements.retain_mut(|p| { - if p.anchor_abs_row < old_live_top { - false - } else { - p.anchor_abs_row -= old_len; - true - } - }); + let old_len = self.collapse_scrollback(); // Same collapse for the selection: a live-area selection // shifts with its rows, one touching cleared history is gone. self.selection = self @@ -794,15 +841,12 @@ impl Screen { // ── edit ───────────────────────────────────────────────────────── pub fn insert_blank_chars(&mut self, n: u16) { - // #TODO(agent): Guard IL/DL outside the horizontal margins, and - // ICH/DCH below the left margin, when cursor addressing stops - // clamping to DECSLRM. self.cursor.pending_wrap = false; let blank = self.blank(); let x = self.cursor.x as usize; let y = self.cursor.y as usize; let right = self.right_margin() as usize; - if x > right { + if x < self.left_margin() as usize || x > right { return; } let len = right + 1 - x; @@ -829,7 +873,7 @@ impl Screen { let x = self.cursor.x as usize; let y = self.cursor.y as usize; let right = self.right_margin() as usize; - if x > right { + if x < self.left_margin() as usize || x > right { return; } let len = right + 1 - x; @@ -870,7 +914,11 @@ impl Screen { pub fn insert_lines(&mut self, n: u16) { self.cursor.pending_wrap = false; - if self.cursor.y < self.region.top || self.cursor.y > self.region.bottom { + if self.cursor.y < self.region.top + || self.cursor.y > self.region.bottom + || self.cursor.x < self.left_margin() + || self.cursor.x > self.right_margin() + { return; } let start = self.cursor.y as usize; @@ -895,7 +943,11 @@ impl Screen { pub fn delete_lines(&mut self, n: u16) { self.cursor.pending_wrap = false; - if self.cursor.y < self.region.top || self.cursor.y > self.region.bottom { + if self.cursor.y < self.region.top + || self.cursor.y > self.region.bottom + || self.cursor.x < self.left_margin() + || self.cursor.x > self.right_margin() + { return; } let start = self.cursor.y as usize; diff --git a/crates/noa-grid/src/screen/print.rs b/crates/noa-grid/src/screen/print.rs index d27814fb..20d84c4f 100644 --- a/crates/noa-grid/src/screen/print.rs +++ b/crates/noa-grid/src/screen/print.rs @@ -5,6 +5,17 @@ use super::*; impl Screen { // ── printing ─────────────────────────────────────────────────── + /// Absolute addressing can place the cursor outside DECSLRM. Text there + /// wraps at the screen edge until the cursor enters the margin interval. + fn print_margins(&self, x: u16) -> (u16, u16) { + let (left, right) = (self.left_margin(), self.right_margin()); + if (left..=right).contains(&x) { + (left, right) + } else { + (0, self.cols.saturating_sub(1)) + } + } + /// [`Screen::print_width`] behind a direct-indexed BMP table. The /// per-scalar `unicode-width` multi-level lookup shows up at ~6% of the /// bulk unicode ingest profile; one byte per BMP codepoint (64 KiB, @@ -62,7 +73,8 @@ impl Screen { return; } - if width == 2 && self.right_margin() <= self.left_margin() { + let (left, right) = self.print_margins(self.cursor.x); + if width == 2 && right <= left { let blank = self.blank(); let (x, y) = (self.cursor.x as usize, self.cursor.y as usize); let Some(row) = self.grid.get_mut(y) else { @@ -80,17 +92,17 @@ impl Screen { row.wrapped = true; } self.index(); - self.cursor.x = self.left_margin(); + self.cursor.x = left; self.cursor.pending_wrap = false; } - if width == 2 && self.cursor.x.saturating_add(1) > self.right_margin() { + if width == 2 && self.cursor.x.saturating_add(1) > right { if autowrap { if let Some(row) = self.grid.get_mut(self.cursor.y as usize) { row.wrapped = true; } self.index(); - self.cursor.x = self.left_margin(); + self.cursor.x = left; self.cursor.pending_wrap = false; } else { let blank = self.blank(); @@ -171,8 +183,8 @@ impl Screen { row.dirty = true; row.mark_occupied(x + width); - if self.cursor.x.saturating_add(width as u16) > self.right_margin() { - self.cursor.x = self.right_margin(); + if self.cursor.x.saturating_add(width as u16) > right { + self.cursor.x = right; self.cursor.pending_wrap = true; // latch; stay in the last column } else { self.cursor.x += width as u16; @@ -191,6 +203,13 @@ impl Screen { bytes.iter().all(|&b| (0x20..=0x7e).contains(&b)), "print_ascii_run only takes printable ASCII" ); + if self.cursor.x < self.left_margin() || self.cursor.x > self.right_margin() { + // Crossing into the margins can change the wrap boundary mid-run. + for &b in bytes { + self.print(b as char, autowrap, grapheme_clustering); + } + return; + } let mut bytes = bytes; if grapheme_clustering { // Only a run prefix can extend a pre-existing cluster (an ASCII @@ -238,16 +257,9 @@ impl Screen { // scalar without moving the cursor, so the whole rest drops. return; } - // Cells available on this row segment: through the right margin - // (inclusive) and within the row; a cursor already past the - // margin still writes one cell before snapping back (as the - // per-scalar path does). + // Cells available through the right margin (inclusive). let seg_end = (right as usize + 1).min(row.cells.len()); - let n = if x >= seg_end { - 1 - } else { - (seg_end - x).min(bytes.len() - i) - }; + let n = (seg_end - x).min(bytes.len() - i); // A wide pair straddling the segment can only leak a stray half // *outside* the segment through its two edge cells — an interior // hit's neighbor is also inside the segment and gets the same @@ -321,6 +333,12 @@ impl Screen { ) where I: Iterator, { + if self.cursor.x < self.left_margin() || self.cursor.x > self.right_margin() { + for c in chars { + self.print(c, autowrap, grapheme_clustering); + } + return; + } let mut chars = chars.peekable(); if grapheme_clustering { // As in `print_ascii_run`: only a run prefix can extend a @@ -552,7 +570,8 @@ impl Screen { /// glyph may overdraw its neighbor, but the grid never desyncs. fn promote_cluster_to_wide(&mut self, x: usize) { let spacer_x = x + 1; - if spacer_x > self.right_margin() as usize { + let (_, right) = self.print_margins(x as u16); + if spacer_x > right as usize { return; } let blank = self.blank(); @@ -582,8 +601,8 @@ impl Screen { }; row.dirty = true; if self.cursor.x as usize == spacer_x { - if self.cursor.x.saturating_add(1) > self.right_margin() { - self.cursor.x = self.right_margin(); + if self.cursor.x.saturating_add(1) > right { + self.cursor.x = right; self.cursor.pending_wrap = true; } else { self.cursor.x += 1; diff --git a/crates/noa-grid/src/tests/bulk_print.rs b/crates/noa-grid/src/tests/bulk_print.rs index 312a4b47..3a3cc299 100644 --- a/crates/noa-grid/src/tests/bulk_print.rs +++ b/crates/noa-grid/src/tests/bulk_print.rs @@ -52,10 +52,25 @@ fn bulk_print_matches_per_scalar_with_autowrap_off() { fn bulk_print_matches_per_scalar_inside_horizontal_margins() { // DECLRMM + DECSLRM 3..6, cursor inside the margins. assert_bulk_print_matches_per_scalar(10, 4, b"\x1b[?69h\x1b[3;6s\x1b[1;4H", "abcdefghijkl"); - // Cursor placed right of the right margin: one write, then snap + latch. + // Cursor right of the margin wraps at the screen edge. assert_bulk_print_matches_per_scalar(10, 4, b"\x1b[?69h\x1b[3;6s\x1b[1;9H", "abc"); } +#[test] +fn bulk_print_matches_per_scalar_across_horizontal_margin_boundaries() { + for col in [1, 2, 3, 7, 8, 9, 10] { + for autowrap in [true, false] { + let setup = format!( + "\x1b[?69h\x1b[3;7s\x1b[?7{}\x1b[1;{col}H", + if autowrap { 'h' } else { 'l' } + ); + for text in ["abcdefghijkl", "日本語日本語", "❤\u{fe0f}AB"] { + assert_bulk_print_matches_per_scalar(10, 6, setup.as_bytes(), text); + } + } + } +} + #[test] fn bulk_print_matches_per_scalar_overwriting_wide_cells() { let setup = "日本語\x1b[1;2H".as_bytes(); diff --git a/crates/noa-grid/src/tests/kitty_graphics.rs b/crates/noa-grid/src/tests/kitty_graphics.rs index 2eec4a63..b56dd0c9 100644 --- a/crates/noa-grid/src/tests/kitty_graphics.rs +++ b/crates/noa-grid/src/tests/kitty_graphics.rs @@ -732,3 +732,48 @@ fn kitty_rectangle_scroll_keeps_placements_outside_the_margins() { ); assert_eq!(t.kitty_visible_placements()[0].grid_y, 5); } + +// N06: the app's "Clear Scrollback" (`Terminal::clear_scrollback`, not +// `ED 3`) must collapse placement anchors with the history it removes, or a +// live-area image jumps `scrollback_len` rows down and off-screen. +#[test] +fn kitty_clear_scrollback_reanchors_live_placements_and_drops_history_ones() { + let mut t = kitty_terminal(); + // One image on the first row, then push it into history with a screenful + // of newlines plus a few more so the scrollback is non-empty. + feed( + &mut t, + &kitty_apc("a=T,f=32,s=10,v=20,i=1,C=1", &vec![0u8; 10 * 20 * 4]), + ); + feed(&mut t, b"\x1b[24;1H"); + feed(&mut t, &[b'\n'; 30]); + assert!( + t.scrollback_len() >= 30, + "the first image is in history now" + ); + // A second image on live row 2. + feed(&mut t, b"\x1b[3;1H"); + feed( + &mut t, + &kitty_apc("a=T,f=32,s=10,v=20,i=2,C=1", &vec![0u8; 10 * 20 * 4]), + ); + let before = t.kitty_visible_placements(); + assert_eq!(before.len(), 1); + assert_eq!((before[0].image_id, before[0].grid_y), (2, 2)); + + t.clear_scrollback(); + + assert_eq!(t.scrollback_len(), 0); + assert_eq!( + t.primary.kitty_placements.len(), + 1, + "the history-anchored image is gone with the history" + ); + let after = t.kitty_visible_placements(); + assert_eq!(after.len(), 1, "the live image is still visible"); + assert_eq!( + (after[0].image_id, after[0].grid_y), + (2, 2), + "the live image keeps its screen row" + ); +} diff --git a/crates/noa-grid/src/tests/terminal_state.rs b/crates/noa-grid/src/tests/terminal_state.rs index 8541cfff..8ea83dce 100644 --- a/crates/noa-grid/src/tests/terminal_state.rs +++ b/crates/noa-grid/src/tests/terminal_state.rs @@ -170,15 +170,19 @@ fn decslrm_requires_left_right_margin_mode() { } #[test] -fn decslrm_sets_horizontal_margins_and_homes_to_left_margin() { +fn decslrm_sets_horizontal_margins_and_homes_the_cursor() { + // xterm/Ghostty: DECSLRM homes to the *screen's* top-left; only DECOM + // makes home margin-relative (N07). let t = run_size(10, 5, b"\x1b[?69h\x1b[3;7s"); assert_eq!( t.primary.horizontal_margins, Some(HorizontalMargins { left: 2, right: 6 }) ); - assert_eq!(t.primary.cursor.x, 2); - assert_eq!(t.primary.cursor.y, 0); + assert_eq!((t.primary.cursor.x, t.primary.cursor.y), (0, 0)); + + let t = run_size(10, 5, b"\x1b[?69h\x1b[?6h\x1b[3;7s"); + assert_eq!((t.primary.cursor.x, t.primary.cursor.y), (2, 0)); } #[test] @@ -195,7 +199,7 @@ fn horizontal_margins_clamp_cursor_motion_and_carriage_return() { #[test] fn horizontal_margins_wrap_printing_to_left_margin() { - let t = run_size(10, 5, b"\x1b[?69h\x1b[3;7sabcdeZ"); + let t = run_size(10, 5, b"\x1b[?69h\x1b[3;7s\x1b[3GabcdeZ"); assert_eq!(row_text(&t, 0, 8), " abcde "); assert_eq!(cell(&t, 2, 1).ch, 'Z'); @@ -1335,3 +1339,205 @@ fn ris_keeps_the_configured_default_cursor_style() { s.feed(b"\x1bc", &mut t); assert_eq!(t.primary.cursor.style, CursorStyle::SteadyBar); } + +// N07: with DECOM off, absolute cursor placement (CUP/CHA/HPA and the home +// DECSLRM performs) is bounded by the screen, not the left/right margins — +// Ghostty's `setCursorPos` only offsets/clamps to the margins in origin mode. +#[test] +fn absolute_cursor_placement_ignores_horizontal_margins_without_origin_mode() { + // DECLRMM on, margins at columns 10..20 (0-based 9..19). + let t = run(b"\x1b[?69h\x1b[10;20s\x1b[1;1H"); + assert_eq!((t.primary.cursor.x, t.primary.cursor.y), (0, 0)); + let t = run(b"\x1b[?69h\x1b[10;20s\x1b[3;5H"); + assert_eq!((t.primary.cursor.x, t.primary.cursor.y), (4, 2)); + let t = run(b"\x1b[?69h\x1b[10;20s\x1b[30G"); + assert_eq!(t.primary.cursor.x, 29); + let t = run(b"\x1b[?69h\x1b[10;20s\x1b[30`"); + assert_eq!(t.primary.cursor.x, 29); + // DECSLRM itself homes to the screen's top-left when origin mode is off. + let t = run(b"\x1b[?69h\x1b[5;5H\x1b[10;20s"); + assert_eq!((t.primary.cursor.x, t.primary.cursor.y), (0, 0)); +} + +#[test] +fn absolute_cursor_placement_is_margin_relative_in_origin_mode() { + let t = run(b"\x1b[?69h\x1b[10;20s\x1b[?6h\x1b[1;1H"); + assert_eq!((t.primary.cursor.x, t.primary.cursor.y), (9, 0)); + let t = run(b"\x1b[?69h\x1b[10;20s\x1b[?6h\x1b[1;30H"); + assert_eq!(t.primary.cursor.x, 19, "clamped to the right margin"); + let t = run(b"\x1b[?69h\x1b[10;20s\x1b[?6h\x1b[5G"); + assert_eq!(t.primary.cursor.x, 13); + let t = run(b"\x1b[?69h\x1b[?6h\x1b[5;5H\x1b[10;20s"); + assert_eq!((t.primary.cursor.x, t.primary.cursor.y), (9, 0)); +} +#[test] +fn edit_commands_ignore_cursor_outside_horizontal_margins() { + for command in [b'@', b'P', b'L', b'M'] { + for col in [1, 2, 8, 10] { + let mut t = Terminal::new(GridSize::new(10, 3)); + let mut stream = Stream::new(); + stream.feed(b"ABCDEFGHIJ\r\nKLMNOPQRST\r\nUVWXYZ1234", &mut t); + stream.feed(b"\x1b[?69h\x1b[3;7s", &mut t); + stream.feed(format!("\x1b[1;{col}H").as_bytes(), &mut t); + let before = t.active().grid.iter().map(|row| row.cells.clone()).collect::>(); + stream.feed(&[0x1b, b'[', command], &mut t); + let after = t.active().grid.iter().map(|row| row.cells.clone()).collect::>(); + assert_eq!(after, before, "command {} at column {col}", command as char); + } + } +} + +#[test] +fn print_outside_horizontal_margins_uses_screen_edge() { + use noa_vt::Handler as _; + for bulk in [false, true] { + for text in ["ABC", "日本"] { + let mut t = Terminal::new(GridSize::new(10, 3)); + let mut stream = Stream::new(); + stream.feed(b"\x1b[?69h\x1b[3;7s\x1b[1;9H", &mut t); + if bulk { + stream.feed(text.as_bytes(), &mut t); + } else { + for c in text.chars() { t.print(c); } + } + assert_eq!(cell(&t, 8, 0).ch, text.chars().next().unwrap()); + if text == "ABC" { + assert_eq!(cell(&t, 9, 0).ch, 'B'); + assert_eq!(cell(&t, 0, 1).ch, 'C'); + } else { + assert!(cell(&t, 9, 0).attrs.contains(CellAttrs::WIDE_SPACER)); + assert_eq!(cell(&t, 0, 1).ch, '本'); + } + assert!(t.active().grid[0].wrapped); + } + } +} + +#[test] +fn print_outside_horizontal_margins_without_wrap_stays_at_screen_edge() { + let t = run_size(10, 3, b"\x1b[?69h\x1b[3;7s\x1b[?7l\x1b[1;9HABC"); + assert_eq!(row_text(&t, 0, 10), " AC"); + assert_eq!((t.primary.cursor.x, t.primary.cursor.y), (9, 0)); + assert!(!t.primary.grid[0].wrapped); +} + +#[test] +fn wrap_outside_horizontal_margins_preserves_scroll_rectangle() { + for text in ["ABC", "日本", "A日"] { + let t = run_size( + 10, + 3, + format!( + "ABCDEFGHIJ\r\nKLMNOPQRST\r\nUVWXYZ1234\x1b[?69h\x1b[3;7s\x1b[3;9H{text}" + ) + .as_bytes(), + ); + assert_eq!(row_text(&t, 0, 10), "ABCDEFGHIJ", "{text}"); + assert_eq!(row_text(&t, 1, 10), "KLMNOPQRST", "{text}"); + assert_eq!( + row_text(&t, 2, 7).chars().skip(2).collect::(), + "WXYZ1", + "{text}" + ); + assert_eq!(t.primary.cursor.y, 2); + assert_eq!(t.primary.scrollback_len(), 0); + } +} + +#[test] +fn vertical_motion_outside_horizontal_margins_does_not_scroll() { + for col in [1, 2, 8, 10] { + for (region, row, command, expected_y) in [ + ("1;3", 3, "\n", 2), + ("1;3", 3, "\x1bD", 2), + ("1;3", 1, "\x1bM", 0), + ("2;3", 2, "\x1bM", 0), + ("1;2", 2, "\n", 2), + ] { + let t = run_size( + 10, + 3, + format!( + "ABCDEFGHIJ\r\nKLMNOPQRST\r\nUVWXYZ1234\x1b[?69h\x1b[3;7s\x1b[{region}r\x1b[{row};{col}H{command}" + ) + .as_bytes(), + ); + assert_eq!(row_text(&t, 0, 10), "ABCDEFGHIJ", "{col}, {command:?}"); + assert_eq!(row_text(&t, 1, 10), "KLMNOPQRST", "{col}, {command:?}"); + assert_eq!(row_text(&t, 2, 10), "UVWXYZ1234", "{col}, {command:?}"); + assert_eq!( + (t.primary.cursor.x, t.primary.cursor.y), + (col - 1, expected_y) + ); + } + } +} + +#[test] +fn vertical_motion_inside_horizontal_margins_scrolls_the_rectangle() { + for col in [3, 7] { + for (row, command, expected) in [ + (3, "\n", ["ABMNOPQHIJ", "KLWXYZ1RST", "UV 234"]), + (1, "\x1bM", ["AB HIJ", "KLCDEFGRST", "UVMNOPQ234"]), + ] { + let t = run_size( + 10, + 3, + format!( + "ABCDEFGHIJ\r\nKLMNOPQRST\r\nUVWXYZ1234\x1b[?69h\x1b[3;7s\x1b[{row};{col}H{command}" + ) + .as_bytes(), + ); + for (y, expected_row) in expected.iter().enumerate() { + assert_eq!(row_text(&t, y, 10), *expected_row, "{col}, {command:?}"); + } + assert_eq!( + (t.primary.cursor.x, t.primary.cursor.y), + (col - 1, row - 1) + ); + } + } +} + +#[test] +fn horizontal_motion_outside_margins_respects_direction_and_screen_edges() { + for (col, command, expected_x) in [ + (1, "\x08", 0), + (2, "\x08", 0), + (1, "\x1b[D", 0), + (2, "\x1b[65535D", 0), + (1, "\x1b[Z", 0), + (2, "\x1b[2Z", 0), + (2, "\r", 0), + (9, "\x1b[C", 9), + (10, "\x1b[C", 9), + (9, "\x1b[65535C", 9), + (9, "\t", 9), + (10, "\x1b[2I", 9), + // Moving toward the margin interval still stops at its far edge. + (1, "\x1b[65535C", 6), + (10, "\x1b[65535D", 2), + (2, "\x1b[2C", 3), + (8, "\x1b[2D", 5), + (4, "\x1b[65535D", 2), + (6, "\x1b[65535C", 6), + ] { + let t = run_size( + 10, + 3, + format!("\x1b[?69h\x1b[3;7s\x1b[2;{col}H{command}").as_bytes(), + ); + assert_eq!(t.primary.cursor.x, expected_x, "column {col}, {command:?}"); + assert_eq!(t.primary.cursor.y, 1); + assert!(!t.primary.cursor.pending_wrap); + } +} + +#[test] +fn grapheme_outside_horizontal_margins_can_expand_to_screen_edge() { + let t = run_size(10, 3, "\x1b[?69h\x1b[3;7s\x1b[?2027h\x1b[1;9H❤\u{fe0f}".as_bytes()); + assert!(cell(&t, 8, 0).attrs.contains(CellAttrs::WIDE)); + assert!(cell(&t, 9, 0).attrs.contains(CellAttrs::WIDE_SPACER)); + assert_eq!((t.primary.cursor.x, t.primary.cursor.y), (9, 0)); + assert!(t.primary.cursor.pending_wrap); +} diff --git a/crates/noa-render/src/lib.rs b/crates/noa-render/src/lib.rs index 9a0a87bd..0010f53f 100644 --- a/crates/noa-render/src/lib.rs +++ b/crates/noa-render/src/lib.rs @@ -31,6 +31,7 @@ pub use instance::{BlendMode, CellInstance, PaneUniformParams, Uniforms, populat pub use renderer::{ ConfirmDialogLayout, PaletteLayout, PaneFrame, Renderer, command_palette_layout, confirm_dialog_layout, paint_startup_frame, renderer_construction_count, + search_prompt_caret_col, }; pub use shared::{GlyphAtlasCache, PipelineCache, SharedGlyphAtlases, SharedPipelines}; pub use snapshot::{ diff --git a/crates/noa-render/src/renderer/mod.rs b/crates/noa-render/src/renderer/mod.rs index 0bcd01a2..829b7e8f 100644 --- a/crates/noa-render/src/renderer/mod.rs +++ b/crates/noa-render/src/renderer/mod.rs @@ -1372,4 +1372,5 @@ use cursor::*; use overlay::*; pub use overlay::{ ConfirmDialogLayout, PaletteLayout, command_palette_layout, confirm_dialog_layout, + search_prompt_caret_col, }; diff --git a/crates/noa-render/src/renderer/overlay.rs b/crates/noa-render/src/renderer/overlay.rs index db0692dd..55d87f25 100644 --- a/crates/noa-render/src/renderer/overlay.rs +++ b/crates/noa-render/src/renderer/overlay.rs @@ -999,6 +999,15 @@ pub(super) fn search_prompt_suffix_cols(buffer: &str, search: &SearchState) -> u search_prompt_suffix(buffer, search).chars().count() } +/// The search prompt's caret column, shared with the OS IME anchor. The +/// prompt is right-aligned and both truncation passes drop from the front, +/// so only the trailing ASCII status separates the caret from the right +/// edge, even for wide or combining query text. If the pane is too narrow +/// to show the caret, anchor at its left edge. +pub fn search_prompt_caret_col(buffer: &str, search: &SearchState, cols: u16) -> u16 { + usize::from(cols).saturating_sub(search_prompt_suffix_cols(buffer, search) + 1) as u16 +} + /// Turn the prompt's display text into row-local [`SegmentCell`]s, one per /// column — a double-width character gets a lead cell plus a blank spacer /// cell (mirroring `noa_grid::Screen`'s WIDE/WIDE_SPACER print path), and a diff --git a/crates/noa-render/src/renderer/tests/overlay.rs b/crates/noa-render/src/renderer/tests/overlay.rs index a33e7097..6db8c6f9 100644 --- a/crates/noa-render/src/renderer/tests/overlay.rs +++ b/crates/noa-render/src/renderer/tests/overlay.rs @@ -31,6 +31,51 @@ fn search_prompt_display_text_reports_no_matches_for_non_empty_query() { assert_eq!(text, "Find: needle\u{258F} no matches"); } +#[test] +fn search_prompt_caret_matches_rendered_cells_after_truncation() { + let mut no_matches = SearchState::default(); + no_matches.set_query( + "needle".to_string(), + Vec::new(), + noa_grid::SearchAnchor::Backward(SelectionPoint::new(0, 0)), + ); + let mut many_matches = SearchState::default(); + many_matches.set_query( + "a".to_string(), + (0..1234) + .map(|row| noa_grid::SearchMatch { + start: SelectionPoint::new(0, row), + end: SelectionPoint::new(0, row), + }) + .collect(), + noa_grid::SearchAnchor::Backward(SelectionPoint::new(0, 1233)), + ); + for search in [SearchState::default(), no_matches, many_matches] { + for buffer in [ + "".to_string(), + "short".to_string(), + "日e\u{301}".repeat(100), + ] { + for cols in [0, 1, 5, 11, 12, 20, 80] { + let text = search_prompt_display_text(&buffer, &search, cols); + let mut cells = search_prompt_segment_cells(&text, [255; 4]); + let excess = cells.len().saturating_sub(usize::from(cols)); + cells.drain(..excess); + let drawn_caret = cells + .iter() + .rposition(|cell| cell.ch == '\u{258F}') + .map_or(0, |index| usize::from(cols) - cells.len() + index); + assert_eq!( + usize::from(search_prompt_caret_col(&buffer, &search, cols)), + drawn_caret, + "buffer {buffer:?}, cols {cols}, status {}", + search_prompt_suffix(&buffer, &search), + ); + } + } + } +} + #[test] fn search_prompt_overlay_emits_top_right_bg_and_glyph_instances_and_tracks_the_buffer() { let Some(mut font) = font_with_rasterized_m() else {