diff --git a/crates/pi-natives/src/text.rs b/crates/pi-natives/src/text.rs index 56de90256..25da9cab1 100644 --- a/crates/pi-natives/src/text.rs +++ b/crates/pi-natives/src/text.rs @@ -23,6 +23,7 @@ const MIN_TAB_WIDTH: u32 = 1; const MAX_TAB_WIDTH: u32 = 16; pub const DEFAULT_TAB_WIDTH: usize = 3; const ESC: u16 = 0x1b; +const OSC8_CLOSE: [u16; 6] = [ESC, b']' as u16, b'8' as u16, b';' as u16, b';' as u16, 0x07]; #[inline] fn clamp_tab_width_for_ops(width: u32) -> usize { @@ -237,6 +238,42 @@ impl AnsiState { } } +#[derive(Default)] +struct WrapState { + sgr: AnsiState, + hyperlink: Option>, +} + +impl WrapState { + #[inline] + const fn new() -> Self { + Self { sgr: AnsiState::new(), hyperlink: None } + } + + #[inline] + fn apply_ansi_u16(&mut self, seq: &[u16]) { + if is_sgr_u16(seq) { + self.sgr.apply_sgr_u16(&seq[2..seq.len() - 1]); + } else if let Some(uri) = osc8_uri_u16(seq) { + if uri.is_empty() { + self.hyperlink = None; + } else { + let hyperlink = self.hyperlink.get_or_insert_default(); + hyperlink.clear(); + hyperlink.extend_from_slice(seq); + } + } + } + + #[inline] + fn write_restore_u16(&self, out: &mut Vec) { + self.sgr.write_restore_u16(out); + if let Some(hyperlink) = &self.hyperlink { + out.extend_from_slice(hyperlink); + } + } +} + #[inline] fn write_color_u16(out: &mut Vec, color: ColorVal, base: u32, first: &mut bool) { if color == COLOR_NONE { @@ -372,6 +409,28 @@ fn is_sgr_u16(seq: &[u16]) -> bool { seq.len() >= 3 && seq[1] == b'[' as u16 && *seq.last().unwrap() == b'm' as u16 } +#[inline] +fn osc8_uri_u16(seq: &[u16]) -> Option<&[u16]> { + if seq.len() < OSC8_CLOSE.len() + || seq[0] != ESC + || seq[1] != b']' as u16 + || seq[2] != b'8' as u16 + || seq[3] != b';' as u16 + { + return None; + } + + let body_end = if seq.last() == Some(&0x07_u16) { + seq.len() - 1 + } else if seq.ends_with(&[ESC, b'\\' as u16]) { + seq.len() - 2 + } else { + return None; + }; + let uri_start = seq[4..body_end].iter().position(|&u| u == b';' as u16)? + 5; + Some(&seq[uri_start..body_end]) +} + struct Osc66Info<'a> { payload: &'a [u16], scale: usize, @@ -852,43 +911,44 @@ fn flush_pending_ansi( // ============================================================================ #[inline] -fn write_active_codes(state: &AnsiState, out: &mut Vec) { - if !state.is_empty() { - state.write_restore_u16(out); +fn write_active_codes(state: &WrapState, out: &mut Vec) { + state.write_restore_u16(out); +} + +#[inline] +fn write_hyperlink_close(state: &WrapState, out: &mut Vec) { + if state.hyperlink.is_some() { + out.extend_from_slice(&OSC8_CLOSE); } } #[inline] -fn write_line_end_reset(state: &AnsiState, out: &mut Vec) { - let has_underline = state.attrs & ATTR_UNDERLINE != 0; - let has_strike = state.attrs & ATTR_STRIKE != 0; - if !has_underline && !has_strike { - return; - } - - out.extend_from_slice(&[ESC, b'[' as u16]); - if has_underline { - out.extend_from_slice(&[b'2' as u16, b'4' as u16]); - if has_strike { - out.push(b';' as u16); +fn write_line_end_reset(state: &WrapState, out: &mut Vec) { + let has_underline = state.sgr.attrs & ATTR_UNDERLINE != 0; + let has_strike = state.sgr.attrs & ATTR_STRIKE != 0; + if has_underline || has_strike { + out.extend_from_slice(&[ESC, b'[' as u16]); + if has_underline { + out.extend_from_slice(&[b'2' as u16, b'4' as u16]); + if has_strike { + out.push(b';' as u16); + } } + if has_strike { + out.extend_from_slice(&[b'2' as u16, b'9' as u16]); + } + out.push(b'm' as u16); } - if has_strike { - out.extend_from_slice(&[b'2' as u16, b'9' as u16]); - } - out.push(b'm' as u16); + write_hyperlink_close(state, out); } -fn update_state_from_text(data: &[u16], state: &mut AnsiState) { +fn update_state_from_text(data: &[u16], state: &mut WrapState) { let mut i = 0usize; while i < data.len() { if data[i] == ESC && let Some(seq_len) = ansi_seq_len_u16(data, i) { - let seq = &data[i..i + seq_len]; - if is_sgr_u16(seq) { - state.apply_sgr_u16(&seq[2..seq_len - 1]); - } + state.apply_ansi_u16(&data[i..i + seq_len]); i += seq_len; continue; } @@ -977,7 +1037,7 @@ fn break_long_word( word: &[u16], width: usize, tab_width: usize, - state: &mut AnsiState, + state: &mut WrapState, ) -> SmallVec<[Vec; 4]> { let mut lines = SmallVec::<[Vec; 4]>::new(); let mut current_line = Vec::::new(); @@ -1004,9 +1064,7 @@ fn break_long_word( continue; } current_line.extend_from_slice(seq); - if is_sgr_u16(seq) { - state.apply_sgr_u16(&seq[2..seq_len - 1]); - } + state.apply_ansi_u16(seq); i += seq_len; continue; } @@ -1070,14 +1128,18 @@ fn wrap_single_line(line: &[u16], width: usize, tab_width: usize) -> SmallVec<[V } if visible_width_u16(line, tab_width) <= width { - return smallvec![line.to_vec()]; + let mut only = line.to_vec(); + let mut state = WrapState::new(); + update_state_from_text(line, &mut state); + write_hyperlink_close(&state, &mut only); + return smallvec![only]; } let tokens = split_into_tokens_with_ansi(line); let mut wrapped = SmallVec::<[Vec; 4]>::new(); let mut current_line = Vec::::new(); let mut current_width = 0usize; - let mut state = AnsiState::new(); + let mut state = WrapState::new(); for token in tokens { let token_width = visible_width_u16(&token, tab_width); @@ -1124,6 +1186,7 @@ fn wrap_single_line(line: &[u16], width: usize, tab_width: usize) -> SmallVec<[V } if !current_line.is_empty() { + write_hyperlink_close(&state, &mut current_line); wrapped.push(current_line); } @@ -1148,7 +1211,7 @@ fn wrap_text_with_ansi_impl( } let mut result = SmallVec::<[Vec; 4]>::new(); - let mut state = AnsiState::new(); + let mut state = WrapState::new(); let mut line_start = 0usize; for i in 0..=text.len() { diff --git a/packages/natives/CHANGELOG.md b/packages/natives/CHANGELOG.md index b394fec68..1859d0742 100644 --- a/packages/natives/CHANGELOG.md +++ b/packages/natives/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixed - Fixed `~` (tilde) not expanding for every element of a brace expansion in the bash tool, so `mkdir -p ~/project/{a,b}` now creates both `a` and `b` under `$HOME/project` instead of leaving a literal `~/project/b` in the working directory ([#5819](https://github.com/can1357/oh-my-pi/issues/5819)). +- Fixed ANSI text wrapping to close and restore OSC 8 hyperlinks at physical line boundaries, preventing link targets from leaking into appended content ([#5885](https://github.com/can1357/oh-my-pi/issues/5885)). ## [17.0.2] - 2026-07-17 diff --git a/packages/tui/CHANGELOG.md b/packages/tui/CHANGELOG.md index 83ea1178c..674d1e41a 100644 --- a/packages/tui/CHANGELOG.md +++ b/packages/tui/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixed - Fixed multiline pastes arriving without bracketed-paste markers (e.g. Cmd+V in the Codex desktop embedded terminal on macOS) being split into one submit per line: `StdinBuffer` now collects adjacent ESC-free, CR/LF-bearing stdin reads in a fixed 10 ms classification window and coalesces three or more lines into one paste event, while ambiguous one-break input (including Enter batched with a following keystroke) is replayed unchanged ([#5841](https://github.com/can1357/oh-my-pi/issues/5841)). +- Fixed wrapped OSC 8 links in Markdown tables making cell padding, separators, and adjacent cells clickable ([#5885](https://github.com/can1357/oh-my-pi/issues/5885)). ## [17.0.2] - 2026-07-17 diff --git a/packages/tui/src/components/markdown.ts b/packages/tui/src/components/markdown.ts index 0d0533f71..36775717f 100644 --- a/packages/tui/src/components/markdown.ts +++ b/packages/tui/src/components/markdown.ts @@ -2190,8 +2190,8 @@ export class Markdown implements Component, NativeScrollbackCommittedRows, Nativ if (token.text === token.href || token.text === hrefForComparison) result += clickableLinkText + stylePrefix; else { - const styledLinkUrl = this.#theme.linkUrl(` (${token.href})`); - result += clickableLinkText + formatHyperlink(styledLinkUrl, token.href) + stylePrefix; + const styledLinkUrl = this.#theme.linkUrl(`(${token.href})`); + result += `${clickableLinkText} ${formatHyperlink(styledLinkUrl, token.href)}${stylePrefix}`; } break; } @@ -2388,7 +2388,14 @@ export class Markdown implements Component, NativeScrollbackCommittedRows, Nativ */ #wrapCellText(text: string, maxWidth: number): string[] { const cellWidth = Math.max(1, maxWidth); - return splitTerminalLines(text).flatMap(line => wrapTextWithAnsi(line, cellWidth)); + // Wrap the whole cell in one call so wrapTextWithAnsi() balances OSC 8 + // hyperlink state across explicit newlines (e.g. `
` rendered as \n); + // per-fragment wrapping would drop the reopened link on later rows. + const wrapped = wrapTextWithAnsi(text, cellWidth); + while (wrapped.length > 1 && wrapped[wrapped.length - 1] === "") { + wrapped.pop(); + } + return wrapped; } /** diff --git a/packages/tui/test/markdown.test.ts b/packages/tui/test/markdown.test.ts index 150a4a88c..9d2843f67 100644 --- a/packages/tui/test/markdown.test.ts +++ b/packages/tui/test/markdown.test.ts @@ -1332,6 +1332,33 @@ bar`, terminalState.hyperlinks = originalHyperlinks; }); + function inspectHyperlinks(line: string): { visible: string; targets: Array } { + let activeTarget: string | null = null; + let visible = ""; + const targets: Array = []; + + for (let i = 0; i < line.length; ) { + if (line.startsWith("\x1b]8;;", i)) { + const terminator = line.indexOf("\x07", i + 5); + activeTarget = line.slice(i + 5, terminator) || null; + i = terminator + 1; + continue; + } + if (line.startsWith("\x1b[", i)) { + i += 2; + while (i < line.length && (line.charCodeAt(i) < 0x40 || line.charCodeAt(i) > 0x7e)) i++; + i++; + continue; + } + + visible += line[i]; + targets.push(activeTarget); + i++; + } + + return { visible, targets }; + } + it("should not duplicate URL for autolinked emails", () => { const markdown = new Markdown("Contact user@example.com for help", 0, 0, defaultMarkdownTheme); @@ -1364,24 +1391,107 @@ bar`, expect(output.includes("\x1b]8;;\x07")).toBeTruthy(); }); - it("should keep wrapped URLs inside a single OSC 8 hyperlink span", () => { + it("should balance the complete OSC 8 target around every wrapped URL fragment", () => { + const url = "https://example.com/really/long/path/that/will/wrap/on/narrow/width"; + const markdown = new Markdown(`Visit ${url} for more`, 0, 0, defaultMarkdownTheme); + + const lines = markdown.render(32); + const linkedLines = lines.filter(line => inspectHyperlinks(line).targets.includes(url)); + expect(linkedLines.length).toBeGreaterThan(1); + for (const line of linkedLines) { + expect(line.split(`\x1b]8;;${url}\x07`)).toHaveLength(2); + expect(line.match(/\x1b\]8;;\x07/g)).toHaveLength(1); + expect(new Set(inspectHyperlinks(line).targets.filter(target => target !== null))).toEqual(new Set([url])); + } + }); + + it("should isolate wrapped OSC 8 links from adjacent table cells", () => { + const issueUrl = "https://github.com/can1357/oh-my-pi/issues/5860"; const markdown = new Markdown( - "Visit https://example.com/really/long/path/that/will/wrap/on/narrow/width for more", + `| Issue | Title | +|---|---| +| [#5860](${issueUrl}) | feat(extensions): expose live service-tier state (/fast) to extensions |`, 0, 0, defaultMarkdownTheme, ); - const lines = markdown.render(32); - expect(lines.length).toBeGreaterThan(1); - const output = lines.join("\n"); - const openMatches = - output.match( - /\x1b\]8;;https:\/\/example\.com\/really\/long\/path\/that\/will\/wrap\/on\/narrow\/width\x07/g, - ) || []; - const closeMatches = output.match(/\x1b\]8;;\x07/g) || []; - expect(openMatches.length).toBe(1); - expect(closeMatches.length).toBeGreaterThan(0); + const lines = markdown.render(80).map(inspectHyperlinks); + const issueRow = lines.find(line => line.visible.includes("#5860")); + expect(issueRow).toBeDefined(); + if (!issueRow) throw new Error("Expected rendered issue row"); + + for (const line of lines) { + for (let i = 0; i < line.visible.length; i++) { + if (line.visible[i] === "|") expect(line.targets[i]).toBeNull(); + } + } + + const labelStart = issueRow.visible.indexOf("#5860"); + const separator = issueRow.visible.indexOf("|", labelStart); + expect(issueRow.targets.slice(labelStart, labelStart + "#5860".length)).toEqual( + new Array("#5860".length).fill(issueUrl), + ); + expect(issueRow.targets.slice(labelStart + "#5860".length, separator)).toEqual( + new Array(separator - labelStart - "#5860".length).fill(null), + ); + + const titleStart = issueRow.visible.indexOf("feat(extensions)"); + expect(issueRow.targets.slice(titleStart, titleStart + "feat(extensions)".length)).toEqual( + new Array("feat(extensions)".length).fill(null), + ); + + const linkedText = lines + .flatMap(line => [...line.visible].filter((_, index) => line.targets[index] === issueUrl)) + .join(""); + expect(linkedText).toContain("#5860"); + expect(linkedText).toContain(issueUrl); + expect(new Set(lines.flatMap(line => line.targets).filter(target => target !== null))).toEqual( + new Set([issueUrl]), + ); + }); + + it("should balance OSC 8 links across explicit newlines in a table cell", () => { + const issueUrl = "https://github.com/can1357/oh-my-pi/issues/5860"; + const markdown = new Markdown( + `| Issue | Title | +|---|---| +| [first
second](${issueUrl}) | plain title cell |`, + 0, + 0, + defaultMarkdownTheme, + ); + + const lines = markdown.render(40).map(inspectHyperlinks); + const firstRow = lines.find(line => line.visible.includes("first")); + const secondRow = lines.find(line => line.visible.includes("second")); + expect(firstRow).toBeDefined(); + expect(secondRow).toBeDefined(); + if (!firstRow || !secondRow) throw new Error("Expected both wrapped label rows"); + + // No cell border or padding may carry the link on either physical row. + for (const line of lines) { + for (let i = 0; i < line.visible.length; i++) { + if (line.visible[i] === "|") expect(line.targets[i]).toBeNull(); + } + } + + // Both label fragments split by
must still target the full URL. + for (const [row, label] of [ + [firstRow, "first"], + [secondRow, "second"], + ] as const) { + const start = row.visible.indexOf(label); + expect(row.targets.slice(start, start + label.length)).toEqual(new Array(label.length).fill(issueUrl)); + const separator = row.visible.indexOf("|", start); + expect(row.targets.slice(start + label.length, separator)).toEqual( + new Array(separator - start - label.length).fill(null), + ); + } + + expect(new Set(lines.flatMap(line => line.targets).filter(target => target !== null))).toEqual( + new Set([issueUrl]), + ); }); it("should show URL for explicit markdown links with different text", () => {