Skip to content
47 changes: 47 additions & 0 deletions crates/editor/src/content/buffer_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13019,6 +13019,53 @@ fn test_clipboard_table_copy_uses_source_offsets_for_later_formatted_cells() {
});
}

#[test]
fn test_clipboard_table_copy_emits_real_newline_for_in_cell_break() {
App::test((), |mut app| async move {
// Product decision (#13732): the two clipboard flavors carry a `<br>` differently.
//
// - The plain-text flavor is *rendered text*: it already strips styling, so an
// authored hard break emits a real `\n`, never the literal HTML string "<br>".
// Injecting HTML the user didn't select would violate that rendered-text contract.
// - The HTML flavor (see serialize_table_cell_inline / selected_text_as_html) carries
// the break as a real `<br>` element, so Warp-to-Warp paste round-trips faithfully.
//
// Accepted cost: a plain-text-only re-paste of this row into a Warp table may split it,
// because the in-cell newline collides with the tab/newline row delimiter. That is a
// standard TSV-class limitation we consciously accept — it is NOT an oversight. Do not
// "fix" it by re-emitting "<br>" here; that regresses the plain-text contract above.
let table_source = "a<br>b\tc";
let markdown = format!("```{TABLE_BLOCK_MARKDOWN_LANG}\n{table_source}\n```\n");
let (buffer, _selection) = Buffer::mock_from_markdown(
&markdown,
None,
Box::new(|_, _| IndentBehavior::Ignore),
&mut app,
);

buffer.update(&mut app, |buffer, _ctx| {
let block_start = buffer.containing_block_start(CharOffset::from(1));
let max_offset = buffer.max_charoffset();

let copied = buffer.clipboard_table_text_in_range(
block_start,
block_start..max_offset,
LineEnding::LF,
);

assert!(
!copied.contains("<br>"),
"plain-text flavor must not contain literal HTML the user didn't select, got {copied:?}"
);
// The in-cell break renders as a real newline; the trailing `\n` terminates the row.
assert_eq!(
copied, "a\nb\tc\n",
"in-cell break should emit a real newline in the plain-text flavor"
);
});
});
}

#[test]
fn test_multiselect_text_styling() {
App::test((), |mut app| async move {
Expand Down
10 changes: 9 additions & 1 deletion crates/editor/src/content/edit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,15 @@ fn measure_table_cells(
let mut line = LayOutArgs::new();
line.highlighted_urls = highlight_urls(&runs);
for run in &runs {
line.layout_run(layout, run, &paragraph_style);
// `layout_run` strips a trailing `\n` (from a raw HTML `<br>`) out of
// `line.text`; an interior `\n` is left in place. Reinsert the break at each run
// boundary where it occurred — a `<br>` can land at a style-run boundary (e.g.
// `**bold**<br>**plain**`), so the break may terminate a non-final run. Tracking
// only the final run's flag would drop such interior breaks and silently collapse
// the cell to one line.
if line.layout_run(layout, run, &paragraph_style) {
line.text.push('\n');
}
}
let text_layout = InlineTextLayoutInput {
text: line.text,
Expand Down
151 changes: 151 additions & 0 deletions crates/editor/src/content/edit_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use warpui_core::{App, SingletonEntity};

use super::{
BlockLocation, LayOutArgs, layout_mermaid_diagram_block, layout_table_block, layout_text_block,
measure_table_cells,
};
use crate::content::buffer::{StyledBufferRun, StyledTextBlock};
use crate::content::edit::{
Expand Down Expand Up @@ -843,6 +844,156 @@ fn test_layout_table_block_caches_cell_text_frames() {
})
}

/// Lays out the given internal-format table string and returns the measured cell text strings
/// (header row first, then body rows). The returned `String`s are the exact inputs handed to the
/// text layout system, so a trailing `\n` here means the cell will render a trailing empty line.
///
/// This asserts on `measure_table_cells`' output rather than laid-out line counts because
/// `App::test`'s text layout backend is a stub (`platform::test::delegate`'s `layout_text` ignores
/// its input and always returns a single empty line), so `line_heights.len()` cannot observe
/// newline handling.
fn measured_cell_texts(text_layout: &TextLayout<'_>, content: &str) -> Vec<Vec<String>> {
let table =
crate::content::text::table_from_internal_format_with_inline_markdown(content, Vec::new());
let table_style = text_layout.rich_text_styles().table_style;
let mut header_style = text_layout.paragraph_styles(&BufferBlockStyle::table(Vec::new()));
header_style.font_weight = Weight::Bold;
let body_style = text_layout.paragraph_styles(&BufferBlockStyle::table(Vec::new()));

let (_widths, cell_text_layouts) =
measure_table_cells(&table, text_layout, &table_style, header_style, body_style);
cell_text_layouts
.into_iter()
.map(|row| row.into_iter().map(|cell| cell.text_layout.text).collect())
.collect()
}

/// A trailing `<br>` at the end of a table cell must preserve the trailing newline in the cell's
/// measured text so it renders an extra empty line, matching GitHub's rendering of a trailing hard
/// break inside a GFM pipe-table cell.
#[test]
fn test_measure_table_cells_trailing_br_keeps_newline() {
App::test((), |app| async move {
app.read(|ctx| {
let layout_cache = LayoutCache::new();
let text_layout = TextLayout::new(
&layout_cache,
ctx.font_cache().text_layout_system(),
&TEST_STYLES,
f32::MAX,
);
// Body cell 0 ends in `<br>`; control cell 1 is a plain single-line cell.
let cells = measured_cell_texts(&text_layout, "a\tb\none<br>\ttwo\n");

assert_eq!(
cells[1][0], "one\n",
"cell ending in <br> must keep its trailing newline (extra empty line)"
);
assert_eq!(
cells[1][1], "two",
"control cell without <br> must be unchanged"
);
});
})
}

/// A cell containing only `<br>` parses to a lone `\n` fragment; its measured text must be a lone
/// newline (an empty content line plus a trailing empty line), not the empty string.
#[test]
fn test_measure_table_cells_lone_br_keeps_newline() {
App::test((), |app| async move {
app.read(|ctx| {
let layout_cache = LayoutCache::new();
let text_layout = TextLayout::new(
&layout_cache,
ctx.font_cache().text_layout_system(),
&TEST_STYLES,
f32::MAX,
);
let cells = measured_cell_texts(&text_layout, "a\tb\n<br>\ttwo\n");

assert_eq!(cells[1][0], "\n", "a lone <br> cell must keep its newline");
});
})
}

/// Embedded `<br>` (text on both sides) is the control path: the interior newline was never
/// stripped, so it must remain untouched by the trailing-break fix.
#[test]
fn test_measure_table_cells_embedded_br_unchanged() {
App::test((), |app| async move {
app.read(|ctx| {
let layout_cache = LayoutCache::new();
let text_layout = TextLayout::new(
&layout_cache,
ctx.font_cache().text_layout_system(),
&TEST_STYLES,
f32::MAX,
);
let cells = measured_cell_texts(&text_layout, "a\tb\none<br>two\tthree\n");

assert_eq!(
cells[1][0], "one\ntwo",
"embedded <br> must keep its single interior newline"
);
});
})
}

/// A `<br>` that falls between two styled runs (e.g. `**bold**<br>**plain**`) parses to a
/// standalone `\n` fragment sitting in a non-final run position. `layout_run` strips that run's
/// trailing `\n` and reports the break, but the following run reports no break — so tracking only
/// the final run's flag would drop the interior newline and collapse the two lines into one. The
/// break must be reinserted per-run, at the run boundary.
#[test]
fn test_measure_table_cells_style_boundary_br_keeps_newline() {
App::test((), |app| async move {
app.read(|ctx| {
let layout_cache = LayoutCache::new();
let text_layout = TextLayout::new(
&layout_cache,
ctx.font_cache().text_layout_system(),
&TEST_STYLES,
f32::MAX,
);
// Body cell 0: bold "bold", a <br> break, bold "plain". The <br> becomes a standalone
// `\n` fragment between the two bold runs — a non-final run that ends in the break.
let cells = measured_cell_texts(&text_layout, "a\tb\n**bold**<br>**plain**\ttwo\n");

assert_eq!(
cells[1][0], "bold\nplain",
"a <br> at a style-run boundary must keep its newline between the runs"
);
});
})
}

/// A cell with breaks in both an interior style-boundary position and a trailing position must
/// preserve every newline, proving the per-run reinsertion handles multiple breaks rather than
/// only the last one.
#[test]
fn test_measure_table_cells_style_boundary_and_trailing_br() {
App::test((), |app| async move {
app.read(|ctx| {
let layout_cache = LayoutCache::new();
let text_layout = TextLayout::new(
&layout_cache,
ctx.font_cache().text_layout_system(),
&TEST_STYLES,
f32::MAX,
);
// Body cell 0: an interior style-boundary break (between the two bold runs) plus a
// trailing break at the end of the cell.
let cells = measured_cell_texts(&text_layout, "a\tb\n**bold**<br>**plain**<br>\ttwo\n");

assert_eq!(
cells[1][0], "bold\nplain\n",
"breaks at a style boundary and at the trailing position must both be preserved"
);
});
})
}

#[test]
fn test_layout_table_block_clamps_cell_width_to_max() {
App::test((), |app| async move {
Expand Down
32 changes: 30 additions & 2 deletions crates/editor/src/content/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1172,7 +1172,25 @@ where
serializer.start_elem(QualName::new(None, ns!(html), "code".into()), iter::empty())?;
}

serializer.write_text(&fragment.text)?;
// An authored hard break (raw HTML `<br>`) is stored as an embedded newline. In an
// HTML table cell a raw newline collapses to a space, silently dropping the break, so
// emit a real `<br>` element between the split parts instead. Inline code cannot
// contain such a newline (the `.md` inline parser never produces one), so it is written
// verbatim. Mirrors the guard in `inline_to_markdown`.
if !styles.is_inline_code() && fragment.text.contains('\n') {
let mut parts = fragment.text.split('\n');
if let Some(first) = parts.next() {
serializer.write_text(first)?;
}
for part in parts {
let br_tag = QualName::new(None, ns!(html), "br".into());
serializer.start_elem(br_tag.clone(), iter::empty())?;
serializer.end_elem(br_tag)?;
serializer.write_text(part)?;
}
} else {
serializer.write_text(&fragment.text)?;
}

if styles.is_inline_code() {
serializer.end_elem(QualName::new(None, ns!(html), "code".into()))?;
Expand Down Expand Up @@ -1249,10 +1267,20 @@ fn inline_to_markdown(inline: &FormattedTextInline) -> String {
let mut previous_styles = TextStylesWithMetadata::default();
for fragment in inline {
let next_styles = TextStylesWithMetadata::from(fragment.styles.clone());
// An authored hard break (raw HTML `<br>`) is stored as an embedded newline. Emit it as
// literal `<br>` so it survives the GFM pipe-table serialization without splitting the
// cell into a spurious extra row. Inline code cannot contain a newline from the `.md`
// inline parser, so it is left untouched. Mirrors the guard in
// `markdown_parser::inline_to_markdown`.
let fragment_text = if !next_styles.is_inline_code() && fragment.text.contains('\n') {
fragment.text.replace('\n', "<br>")
} else {
fragment.text.clone()
};
let content = BufferMarkdownParser::append_formatting(
&previous_styles,
&next_styles,
fragment.text.as_str(),
fragment_text.as_str(),
&mut markdown,
);
previous_styles = next_styles;
Expand Down
63 changes: 63 additions & 0 deletions crates/editor/src/content/markdown_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,40 @@ fn test_gfm_table_html_serialization() {
});
}

#[test]
fn test_gfm_table_html_serialization_preserves_br_in_cell() {
App::test((), |mut app| async move {
let _flag = warp_core::features::FeatureFlag::MarkdownTables.override_enabled(true);
let markdown = "\
| header 1 | header 2 |\n\
| --- | --- |\n\
| line one<br>line two | value 2 |\n";
let (buffer, _selection) = Buffer::mock_from_markdown(
markdown,
None,
Box::new(|_, _| IndentBehavior::Ignore),
&mut app,
);

let html = app.read_model(&buffer, |buffer, ctx| {
let range = CharOffset::from(1)..buffer.max_charoffset();
buffer.ranges_as_html(Vec1::try_from_vec(vec![range]).unwrap(), ctx)
});

let html = html.expect("table should export HTML");
// The in-cell break must serialize as a real <br> element, not a raw newline (which
// HTML collapses to a space, silently dropping the break).
assert!(
html.contains("<td align=\"left\">line one<br>line two</td>"),
"cell break should become a <br> element, got {html}"
);
assert!(
!html.contains("line one\nline two"),
"cell HTML must not contain a raw newline, got {html:?}"
);
});
}

#[test]
fn test_apply_formatted_text_delta_append() {
App::test((), |mut app| async move {
Expand Down Expand Up @@ -357,6 +391,35 @@ fn test_table_markdown_export_escapes_pipe_characters() {
});
}

#[test]
fn test_table_markdown_export_preserves_br_line_breaks() {
App::test((), |mut app| async move {
// A cell whose text contains a `<br>` hard break is stored internally as an
// embedded newline. The GFM export must emit it back as literal `<br>` so the
// newline does not split the pipe-table row into a spurious extra row.
let markdown = format!(
"```{}\nheader 1\theader 2\nline one<br>line two\tvalue 2\n```\n",
TABLE_BLOCK_MARKDOWN_LANG
);
let (buffer, _selection) = Buffer::mock_from_markdown(
&markdown,
None,
Box::new(|_, _| IndentBehavior::Ignore),
&mut app,
);

let exported_markdown = app.read_model(&buffer, |buffer, _| buffer.markdown_unescaped());
assert_eq!(
exported_markdown,
"| header 1 | header 2 |\n| --- | --- |\n| line one<br>line two | value 2 |\n"
);
assert!(
!exported_markdown.contains("line one\nline two"),
"in-cell hard break must not emit a raw newline that splits the table row: {exported_markdown:?}"
);
});
}

#[test]
fn test_url_link_display_text_round_trip_is_stable() {
App::test((), |mut app| async move {
Expand Down
Loading