diff --git a/Cargo.lock b/Cargo.lock index 8296b07..7d2c9ec 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1654,6 +1654,7 @@ dependencies = [ "ghostty-datastruct", "ghostty-font", "ghostty-input", + "ghostty-onig", "ghostty-os", "ghostty-terminal", "ghostty-unicode", diff --git a/crates/ghostty-core/src/surface/mouse.rs b/crates/ghostty-core/src/surface/mouse.rs index f7d73bf..48016af 100644 --- a/crates/ghostty-core/src/surface/mouse.rs +++ b/crates/ghostty-core/src/surface/mouse.rs @@ -49,6 +49,10 @@ use ghostty_vt::modes::Mode; use super::{MOUSE_BUTTON_COUNT, Surface}; use crate::surface_mouse; +/// Bound synchronous link lookup while the terminal mutex is held. Oversized +/// logical lines are skipped, rather than truncating a URL into another target. +const MAX_LINK_CELLS: usize = 64 * 1024; + /// Mouse state for the surface (Zig: `Surface.Mouse`). pub struct Mouse { /// The last tracked mouse button state by button. @@ -1570,35 +1574,37 @@ impl Surface { mouse_mods: Option, ) -> Option { let cfg = self.cfg(); - if cfg.links.is_empty() { + // Check eligibility before walking scrollback or allocating a string + // map. Ordinary mouse movement needs no URL lookup without Ctrl/Super. + let mut links = cfg + .links + .iter() + .filter(|link| { + mouse_mods.is_none_or(|mods| match link.highlight { + Highlight::Always | Highlight::Hover => true, + Highlight::AlwaysMods(v) | Highlight::HoverMods(v) => v.equal(mods), + }) + }) + .peekable(); + if links.peek().is_none() { return None; } - let line = screen.select_line(SelectLine { - pin: mouse_pin, - whitespace: None, - semantic_prompt_boundary: false, - })?; + let line = screen.select_line_bounded( + SelectLine { + pin: mouse_pin, + whitespace: None, + semantic_prompt_boundary: false, + }, + MAX_LINK_CELLS / usize::from(screen.pages.cols), + )?; let strmap = screen.selection_string_map(SelectionString { sel: line, trim: false, }); - for link in &cfg.links { - // Skip highlight/mods check when mouse_mods is None - // (double-click mode) - if let Some(mods) = mouse_mods { - match link.highlight { - Highlight::Always | Highlight::Hover => {} - Highlight::AlwaysMods(v) | Highlight::HoverMods(v) => { - if !v.equal(mods) { - continue; - } - } - } - } - + for link in links { let mut it = strmap.search_iterator(&link.regex); while let Some(m) = it.next_match() { let sel = m.selection(); diff --git a/crates/ghostty-core/tests/e2e_mouse.rs b/crates/ghostty-core/tests/e2e_mouse.rs index 06160bc..7a2c25b 100644 --- a/crates/ghostty-core/tests/e2e_mouse.rs +++ b/crates/ghostty-core/tests/e2e_mouse.rs @@ -342,6 +342,43 @@ fn regex_url_ctrl_click_opens_and_plain_click_does_not() { }); } +#[test] +fn long_unwrapped_output_allows_mouse_motion_and_new_tab_shortcut() { + let h = harness!(Harness::recorder("", &[])); + let (cols, rows) = h.grid(); + { + let mut t = h.surface.lock_terminal(); + t.full_reset(); + // One logical line extending far beyond the viewport into history, + // as produced by base64 -w0. Keep an otherwise valid URL at its tail. + let mut bytes = vec![b'a'; usize::from(cols) * 4096]; + bytes.extend_from_slice(b"https://example.com"); + t.vt_stream().next_slice(&bytes).unwrap(); + } + h.clear_actions(); + for mods in [none(), ctrl(), ctrl_shift()] { + h.move_to(5, u32::from(rows - 1), Some(mods)); + h.move_to(6, u32::from(rows - 1), Some(mods)); + } + h.key(Key::KeyT, ctrl_shift(), "T"); + assert!(h.has_action(|a| matches!(a, Action::NewTab))); + assert!(!h.has_action(|a| matches!(a, Action::MouseOverLink(l) if !l.url.is_empty()))); + + // A hard newline bounds the next URL normally; skipping an oversized + // logical line must not disable link detection on subsequent output. + { + let mut t = h.surface.lock_terminal(); + t.vt_stream() + .next_slice(b"\r\nhttps://example.com") + .unwrap(); + } + h.move_to(7, u32::from(rows - 1), Some(ctrl())); + h.wait_action("URL hover after long output", |a| match a { + Action::MouseOverLink(l) if l.url.to_text_lossy() == "https://example.com" => Some(()), + _ => None, + }); +} + #[test] fn middle_click_pastes_selection_clipboard() { let h = harness!(Harness::recorder("", &[])); diff --git a/crates/ghostty-renderer/Cargo.toml b/crates/ghostty-renderer/Cargo.toml index bc3a51d..44cce2c 100644 --- a/crates/ghostty-renderer/Cargo.toml +++ b/crates/ghostty-renderer/Cargo.toml @@ -13,6 +13,7 @@ ghostty-os.workspace = true ghostty-datastruct.workspace = true ghostty-font.workspace = true ghostty-input.workspace = true +ghostty-onig.workspace = true ghostty-terminal.workspace = true ghostty-unicode.workspace = true ghostty-vt.workspace = true diff --git a/crates/ghostty-renderer/src/link.rs b/crates/ghostty-renderer/src/link.rs index b9638e1..388d01f 100644 --- a/crates/ghostty-renderer/src/link.rs +++ b/crates/ghostty-renderer/src/link.rs @@ -7,7 +7,12 @@ use ghostty_input::{ link::{Highlight, Link as InputLink}, }; use ghostty_terminal::{point::Coordinate, render::RenderState}; -use onig::{Regex, Region, SearchOptions}; +use onig::{MatchParam, Regex, Region, SearchOptions}; + +/// Match the whole-search work budget used by terminal StringMap searches. +/// Renderer searches also need a limit: a busy matcher holds the renderer +/// thread's state lock and can block tab switches and renderer shutdown. +const ONI_SEARCH_RETRY_LIMIT: u32 = 100_000; #[derive(Debug, thiserror::Error)] pub enum Error { @@ -103,16 +108,20 @@ impl Set { while offset < str.len() { let haystack = &str[offset..]; let mut region = Region::new(); - let found = link.regex.search_with_options( + let mut param = MatchParam::default(); + ghostty_onig::set_retry_limit_in_search(&mut param, ONI_SEARCH_RETRY_LIMIT); + let found = link.regex.search_with_param( haystack, 0, haystack.len(), SearchOptions::SEARCH_OPTION_NONE, Some(&mut region), + param, ); // A mismatch (or any search failure such as hitting the // retry limit) ends the search for this link. - let Some((offset_start, offset_end)) = found.and(region.pos(0)) else { + let Some((offset_start, offset_end)) = found.ok().flatten().and(region.pos(0)) + else { break; }; @@ -284,4 +293,24 @@ mod tests { fn invalid_regex_is_error() { assert!(Set::from_config(&[link("(", Highlight::Always)]).is_err()); } + + #[test] + fn render_cell_map_limits_backtracking_and_continues_with_other_links() { + let mut t = Terminal::new(Options::new(80, 1)).unwrap(); + // A long near-match must not monopolize the renderer thread. In a + // tab switch the main thread may need that thread's locks to proceed. + let text = format!("{}b", "a".repeat(60)); + t.vt_stream().next_slice(text.as_bytes()).unwrap(); + let mut state = RenderState::new(); + state.update(&mut t).unwrap(); + let set = Set::from_config(&[ + link("(a+)+$", Highlight::Always), + link("b", Highlight::Always), + ]) + .unwrap(); + let mut result = HashSet::new(); + set.render_cell_map(&mut result, &state, None, Mods::default()) + .unwrap(); + assert_eq!(result, HashSet::from([c(60, 0)])); + } } diff --git a/crates/ghostty-terminal/src/screen.rs b/crates/ghostty-terminal/src/screen.rs index c2310eb..180e201 100644 --- a/crates/ghostty-terminal/src/screen.rs +++ b/crates/ghostty-terminal/src/screen.rs @@ -2653,7 +2653,16 @@ impl Screen { /// over whitespace but the line has non-whitespace characters elsewhere, the /// line will be selected. pub fn select_line(&self, opts: SelectLine<'_>) -> Option { + self.select_line_bounded(opts, usize::MAX) + } + + /// Select a complete logical line only if it spans at most `max_rows` + /// physical rows. This bounds interactive link lookup on unbroken output + /// without truncating a URL into a different, clickable target. Explicit + /// line selection uses [`Self::select_line`] and remains unrestricted. + pub fn select_line_bounded(&self, opts: SelectLine<'_>, max_rows: usize) -> Option { let list = &self.pages; + let mut remaining = max_rows.checked_sub(1)?; // The hovered row. // Get the current point semantic prompt state since that determines // boundary conditions too. This makes it so that line selection can @@ -2698,6 +2707,8 @@ impl Screen { break 'start_pin copy; } + remaining = remaining.checked_sub(1)?; + if let Some(v) = semantic_prompt_state { // We need to check every cell in this row in reverse // order since we're going up and back. @@ -2726,6 +2737,9 @@ impl Screen { // The real end of the row is the final row in the soft-wrap. let end_pin: Pin = 'end_pin: { for p in opts.pin.row_iterator(list, Direction::RightDown, None) { + if p.node != opts.pin.node || p.y != opts.pin.y { + remaining = remaining.checked_sub(1)?; + } let row = self.pin_row(p); let cols = self.pin_cols(p); diff --git a/crates/ghostty-terminal/src/screen/tests/p4.rs b/crates/ghostty-terminal/src/screen/tests/p4.rs index bcb4380..3c3487c 100644 --- a/crates/ghostty-terminal/src/screen/tests/p4.rs +++ b/crates/ghostty-terminal/src/screen/tests/p4.rs @@ -292,6 +292,39 @@ fn screen_select_line_across_full_soft_wrap() { expect_sel(&s, &sel, Tag::Screen, (0, 0), (4, 1)); } +#[test] +fn screen_select_line_bounded_counts_both_directions_and_history() { + let mut s = init(4, 3, 100_000); + s.test_write_string("abcdefghijklmnopqrstuvwx").unwrap(); + for y in 0..6 { + let opts = SelectLine { + pin: pin(&s, screen_pt(1, y)), + whitespace: None, + semantic_prompt_boundary: false, + }; + assert!(s.select_line_bounded(opts, 0).is_none()); + assert!(s.select_line_bounded(opts, 5).is_none()); + assert_eq!(s.select_line_bounded(opts, 6), s.select_line(opts)); + assert!(s.select_line_bounded(opts, 6).is_some()); + } +} + +#[test] +fn screen_select_line_bounded_stops_at_hard_line_boundary() { + let mut s = init(4, 5, 0); + s.test_write_string("HEAD\nabcdefghijkl\nTAIL").unwrap(); + for y in 1..4 { + let opts = SelectLine { + pin: pin(&s, screen_pt(1, y)), + whitespace: None, + semantic_prompt_boundary: false, + }; + let sel = s.select_line_bounded(opts, 3).unwrap(); + assert_eq!(sel, s.select_line(opts).unwrap()); + assert!(s.select_line_bounded(opts, 2).is_none()); + } +} + #[test] fn screen_select_line_across_soft_wrap_ignores_blank_lines() { let mut s = init(5, 10, 0); diff --git a/docs/rewrite/ARCHITECTURE.md b/docs/rewrite/ARCHITECTURE.md index 54d0d80..160ce0f 100644 --- a/docs/rewrite/ARCHITECTURE.md +++ b/docs/rewrite/ARCHITECTURE.md @@ -132,6 +132,13 @@ GTK invokes `glib-compile-resources` directly so generated XML/UI files in the runtime again if work remains. Continuous renderer notifications must yield to GTK input, timers, and drawing instead of extending one tick indefinitely. + Interactive link lookup checks modifier eligibility before scanning text. + `Surface::link_at_pin` uses `Screen::select_line_bounded` to skip logical + lines spanning more than 65,536 grid cells, bounding synchronous scrollback + traversal and string-map allocation while holding the terminal mutex. + Explicit text selection remains unrestricted. Renderer link regex searches + use the same 100,000-step whole-search retry budget as terminal `StringMap` + and treat regex failures as no further matches instead of panicking. 9. **Config:** same file format, same keys, same defaults, same CLI flags, same docs text (used by `+show-config --docs` and `--help`). 10. **Tests:** every Zig `test "..."` block in a ported file is ported to a diff --git a/docs/rewrite/DEVIANT_FIXES.md b/docs/rewrite/DEVIANT_FIXES.md index 68825c4..ab90bf1 100644 --- a/docs/rewrite/DEVIANT_FIXES.md +++ b/docs/rewrite/DEVIANT_FIXES.md @@ -65,7 +65,8 @@ when this file was written. | 37 | Termio: subprocess cleanup | `termio/Exec.zig:1188-1224` (`getpgid`) | Loops forever while `getpgid(child) == our pgid`, i.e. also when the child died before `setsid` and is an unreaped zombie (its pgid stays ours), hanging the surface close. | Still waits without a cap (no 5 s limit) while the child is alive, but stops (no SIGHUP, nothing to kill) once `waitid(WNOWAIT)` shows the child exited. | `b9ad595` fix-deviant(termio) | `getpgid_gives_up_on_exited_child_without_setsid`, `getpgid_waits_for_late_setsid` (`ghostty-termio/src/exec.rs`) | | 38 | Config (Windows): absolute theme paths | `config/Config.zig:9788-9799` (`Theme.parseCLI`) | Any value containing `,`, `=` or `:` is parsed as a `light:..,dark:..` pair, and a Windows absolute path (`C:\\themes\\x`, `\\\\?\\C:\\...`) always contains a colon, so `theme = ` is rejected on Windows (found by running the config tests under Wine). | On Windows a value that starts like an absolute Windows path (drive letter + `:` + separator, or `\\\\`) is a single theme; the pair syntax is unchanged (quote backslashes). Other platforms are unchanged. | `97a262d` fix-deviant(config) | `windows_absolute_path_detection`, `theme_windows_absolute_path_and_pair_with_paths` (`ghostty-config/src/types_b.rs`), `theme_loading` under Wine | | 39 | Termio (Windows): bare program names | `Command.zig:261,347-358` (`startWindows`, `CreateProcessW(application_w, ...)`) with `termio/Exec.zig:1004` (`.path = self.args[0]`) | `lpApplicationName` is never searched on `PATH` (a partial name only completes against the current directory), so a `direct:` command / `-e` argv[0] such as `cmd.exe` fails with `FileNotFound`; only the default shell works (it is wrapped as `%WINDIR%\\System32\\cmd.exe /C ...`). | A name without drive/separator is looked up on `PATH` (`.exe` implied) before `CreateProcessW`; failures keep the original name (same error as Zig). | `4d7ad7c` fix-deviant(termio) | `resolve_application_searches_path_for_bare_names`, `bare_program_name_starts_via_path_search` (`ghostty-termio/src/command_windows.rs`), `echo_output_reaches_terminal_and_exit_is_reported` (`tests/exec_windows.rs`, Wine) | -| 40 | App: mailbox starvation | `App.zig:238-266` (`drainMailbox`) | Drains until the mailbox is empty. A producer that keeps refilling it can keep one app tick running indefinitely, preventing the runtime from dispatching input and timers. | Handles at most one mailbox capacity (64 messages) per tick, then wakes the runtime if messages remain. This bounds queue processing without dropping or reordering messages. The regression uses finite callback-driven refills; the reported full freeze with continuous base64 is not consistently reproduced in headless GTK. | this commit (`fix-deviant(core)`) | `app::tests::mailbox_tick_yields_and_rearms_when_producer_keeps_refilling` (`ghostty-core/src/app.rs`) | +| 40 | App: mailbox starvation | `App.zig:238-266` (`drainMailbox`) | Drains until the mailbox is empty. A producer that keeps refilling it can keep one app tick running indefinitely, preventing the runtime from dispatching input and timers. | Handles at most one mailbox capacity (64 messages) per tick, then wakes the runtime if messages remain. This bounds queue processing without dropping or reordering messages. The regression uses finite callback-driven refills. Follow-up user testing confirmed this alone does not fix the base64 hang; see entry 41 for the mouse/link path. | `4c31d27` | `app::tests::mailbox_tick_yields_and_rearms_when_producer_keeps_refilling` (`ghostty-core/src/app.rs`) | +| 41 | Mouse/link lookup under unbroken output | `Surface.zig:4431-4475` (`linkAtPin`), `renderer/link.zig:98-106` (regex search) | Mouse lookup selects and copies the entire soft-wrapped logical line before checking modifier eligibility, so newline-free output can make mouse motion process all scrollback under the terminal mutex. Renderer regex searches have no explicit whole-search retry budget. | Check eligible links first; skip logical lines exceeding 65,536 grid cells using bounded row traversal. Limit renderer regex searches to 100,000 retry steps, and handle errors without panicking (the old Rust search API panicked on retry errors). Explicit line selection stays unrestricted; oversized logical lines are not URL targets. | this commit (`fix-deviant(links)`) | `screen_select_line_bounded_*`, `long_unwrapped_output_allows_mouse_motion_and_new_tab_shortcut`, `render_cell_map_limits_backtracking_and_continues_with_other_links` | ## Upstream quirks deliberately kept (not fixed)