Merge PR #5887: fix(tui): isolate wrapped OSC 8 links in tables (@roboomp)
This commit is contained in:
@@ -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<Vec<u16>>,
|
||||
}
|
||||
|
||||
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<u16>) {
|
||||
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<u16>, 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<u16>) {
|
||||
if !state.is_empty() {
|
||||
state.write_restore_u16(out);
|
||||
fn write_active_codes(state: &WrapState, out: &mut Vec<u16>) {
|
||||
state.write_restore_u16(out);
|
||||
}
|
||||
|
||||
#[inline]
|
||||
fn write_hyperlink_close(state: &WrapState, out: &mut Vec<u16>) {
|
||||
if state.hyperlink.is_some() {
|
||||
out.extend_from_slice(&OSC8_CLOSE);
|
||||
}
|
||||
}
|
||||
|
||||
#[inline]
|
||||
fn write_line_end_reset(state: &AnsiState, out: &mut Vec<u16>) {
|
||||
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<u16>) {
|
||||
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<u16>; 4]> {
|
||||
let mut lines = SmallVec::<[Vec<u16>; 4]>::new();
|
||||
let mut current_line = Vec::<u16>::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<u16>; 4]>::new();
|
||||
let mut current_line = Vec::<u16>::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<u16>; 4]>::new();
|
||||
let mut state = AnsiState::new();
|
||||
let mut state = WrapState::new();
|
||||
let mut line_start = 0usize;
|
||||
|
||||
for i in 0..=text.len() {
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
@@ -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. `<br>` 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;
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -1332,6 +1332,33 @@ bar`,
|
||||
terminalState.hyperlinks = originalHyperlinks;
|
||||
});
|
||||
|
||||
function inspectHyperlinks(line: string): { visible: string; targets: Array<string | null> } {
|
||||
let activeTarget: string | null = null;
|
||||
let visible = "";
|
||||
const targets: Array<string | null> = [];
|
||||
|
||||
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<br>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 <br> 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", () => {
|
||||
|
||||
Reference in New Issue
Block a user