Merge PR #4217: M7: edited text keeps every space typed (#4199, #3797)

crates/edit/src/tests.rs: both sides appended tests at the end; kept both.
Cargo.lock: unchanged from base (the PR's ctutils bump did not apply).
This commit is contained in:
aiongg
2026-10-24 06:22:45 +00:00
4 changed files with 297 additions and 28 deletions
+181
View File
@@ -6873,3 +6873,184 @@ fn a_dash_or_cap_operator_partway_through_an_outlined_line_starts_a_run() {
assert_eq!(shows.iter().map(|s| s.text.as_str()).collect::<String>(), "FAINTS", "{change}");
}
}
/// #4199, #3797: every space typed into a paragraph is saved, not folded into one.
#[test]
fn an_edited_paragraph_keeps_every_space_typed() {
for typed in ["Hello world", "Hello world end", " indented and trailing "] {
let mut doc = text_page("BT /F1 12 Tf 72 700 Td (Hello world) Tj ET");
assert_eq!(text::replace_block(&mut doc, 0, 0, typed).unwrap().substituted, None);
let doc = reopen(&doc);
let content = String::from_utf8_lossy(&page_content_bytes(&doc, 0)).into_owned();
assert!(content.contains(&format!("({typed}) Tj")), "{typed:?}: {content}");
assert_eq!(text::text_lines(&doc, 0).unwrap()[0].text, typed);
}
}
/// Spaces after a row's last word are kept but take no part in its alignment: a justified row
/// still reaches the margin, and a centred or right-aligned one sits where it would without them.
#[test]
fn spaces_after_a_rows_last_word_do_not_move_its_alignment() {
let shown = |text: &str, align: added::Align| {
let mut doc = text_page("BT /F1 12 Tf 72 700 Td (Hello world) Tj ET");
let style =
text::BlockStyle { family: Some((added::Family::Helvetica, false, false)), align: Some(align), width: Some(300.0), ..Default::default() };
text::rewrite_block(&mut doc, 0, 0, Some(text), &style).unwrap();
String::from_utf8_lossy(&page_content_bytes(&doc, 0)).into_owned()
};
for (plain, trailing, align) in [
("aaa bbb ccc\nddd eee", "aaa bbb ccc \nddd eee", added::Align::Justify),
("Title", "Title ", added::Align::Center),
("Title", "Title ", added::Align::Right),
] {
let first = trailing.split('\n').next().unwrap();
let expected = shown(plain, align);
let actual = shown(trailing, align).replace(&format!("({first})"), &format!("({})", first.trim_end()));
assert_eq!(actual, expected, "{trailing:?} {align:?}");
}
}
/// A justified row runs from the paragraph's left edge to the margin. Word spacing reaches only
/// the spaces between its words: the spaces before its first word and after its last are left
/// out of a stretched row (as before #4199), so the reopened rows stay inside the paragraph and
/// can be edited again in place.
#[test]
fn a_justified_row_keeps_its_indent_and_ends_at_the_margin() {
let style = text::BlockStyle {
family: Some((added::Family::Helvetica, false, false)),
align: Some(added::Align::Justify),
width: Some(300.0),
..Default::default()
};
for typed in [" Pack my box with five dozen jugs\nof liquor", " Pack my box with five dozen jugs \nof liquor"] {
let mut doc = text_page("BT /F1 12 Tf 72 700 Td (Hello world) Tj ET");
text::rewrite_block(&mut doc, 0, 0, Some(typed), &style).unwrap();
let mut doc = reopen(&doc);
let lines = text::text_lines(&doc, 0).unwrap();
let first = lines.iter().find(|l| l.text.contains("Pack")).unwrap();
assert_eq!(first.text, "Pack my box with five dozen jugs", "{typed:?}");
assert!((first.rect[0] - 72.0).abs() < 0.5, "{typed:?}: {:?}", first.rect);
assert!(lines.iter().all(|l| l.rect[0] >= 72.0 - 0.5 && l.rect[2] <= 372.5), "{typed:?}: {lines:?}");
let blocks = text::text_blocks(&doc, 0).unwrap();
assert_eq!(blocks.len(), 1, "{typed:?}: {blocks:?}");
text::replace_block(&mut doc, 0, 0, "Edited again").unwrap();
let again = text::text_lines(&reopen(&doc), 0).unwrap();
assert!(again.iter().all(|l| l.rect[0] >= 0.0 && l.rect[2] <= 600.0), "{typed:?}: {again:?}");
}
}
/// A justified paragraph whose first row was typed with an indent is edited again through the
/// styled path (a draft with a line break): its rows keep their scaling and stay one paragraph
/// inside its width.
#[test]
fn a_justified_paragraph_with_an_indent_edits_again_at_its_own_scale() {
let style = text::BlockStyle {
family: Some((added::Family::Helvetica, false, false)),
align: Some(added::Align::Justify),
width: Some(300.0),
..Default::default()
};
let mut doc = text_page("BT /F1 12 Tf 72 700 Td (Hello world) Tj ET");
text::rewrite_block(&mut doc, 0, 0, Some(" aaa bbb ccc \nddd"), &style).unwrap();
let mut doc = reopen(&doc);
let draft = text::text_blocks(&doc, 0).unwrap()[0].editor_text.replacen("ccc", "cc", 1);
assert!(draft.contains('\n'), "{draft:?}");
text::replace_block(&mut doc, 0, 0, &draft).unwrap();
let content = String::from_utf8_lossy(&page_content_bytes(&doc, 0)).into_owned();
let scales: Vec<&str> = content.lines().filter(|l| l.ends_with(" Tz")).collect();
assert!(scales.iter().all(|l| *l == "100 Tz"), "{scales:?}: {content}");
let doc = reopen(&doc);
let blocks = text::text_blocks(&doc, 0).unwrap();
assert_eq!(blocks.len(), 1, "{blocks:?}");
let lines = text::text_lines(&doc, 0).unwrap();
assert!(lines.iter().all(|l| l.rect[0] >= 72.0 - 0.5 && l.rect[2] <= 372.5), "{lines:?}");
}
/// A paragraph drawn at `0 Tz` has no width to justify into: nothing is divided by it. The
/// writer writes a number that isn't finite as 0, so the test looks for the word spacing and
/// the move a division by zero would add, rather than for `NaN`.
#[test]
fn justifying_text_at_zero_horizontal_scaling_divides_by_nothing() {
let mut doc = text_page("BT /F1 12 Tf 0 Tz 72 700 Td (Hello world) Tj ET");
let style = text::BlockStyle { family: Some((added::Family::Helvetica, false, false)), align: Some(added::Align::Justify), ..Default::default() };
text::rewrite_block(&mut doc, 0, 0, Some(" aaa bbb\nccc"), &style).unwrap();
let content = String::from_utf8_lossy(&page_content_bytes(&doc, 0)).into_owned();
let block = content.split("ET").next().unwrap();
assert_eq!(block.matches(" Tw").count(), 1, "only the paragraph's own 0 Tw: {content}");
let first_row = block.split(" Tm").nth(1).and_then(|rest| rest.split(" Tj").next()).unwrap();
assert!(!first_row.contains(" Td"), "no move before the first row: {content}");
}
/// The preview's box is the box of the rows Apply writes, measured to their last glyphs.
#[test]
fn a_drafts_box_is_the_box_apply_draws() {
let doc = text_page("BT /F1 12 Tf 14 TL 72 700 Td [(First) -2000 (row here )] TJ T* (Second row ) Tj ET");
let source = text::text_blocks(&doc, 0).unwrap().swap_remove(0);
let draft = source.editor_text.replacen("Second", "Third", 1);
let layout = text::text_edit_layout(&doc, 0, 0, &draft, &text::BlockStyle::default()).unwrap();
let mut applied = doc.clone();
text::replace_block(&mut applied, 0, 0, &draft).unwrap();
let boxed = text::text_lines(&applied, 0)
.unwrap()
.into_iter()
.map(|l| {
let end = l.ink.iter().map(|r| r[2]).fold(f64::NEG_INFINITY, f64::max);
[l.rect[0], l.rect[1], if end.is_finite() { end.min(l.rect[2]) } else { l.rect[2] }, l.rect[3]]
})
.reduce(|a, r| [a[0].min(r[0]), a[1].min(r[1]), a[2].max(r[2]), a[3].max(r[3])]);
let (d, b) = (layout.draft_rect.unwrap(), boxed.unwrap());
assert!(d.iter().zip(b).all(|(x, y)| (x - y).abs() < 0.01), "{draft:?}: preview {d:?}, applied {b:?}");
}
/// The draft's box is measured to its last glyph, not over the spaces typed after it.
#[test]
fn a_drafts_box_leaves_out_spaces_after_the_last_word() {
let doc = text_page("BT /F1 12 Tf 72 700 Td (Hello world) Tj ET");
let layout = |text: &str| text::text_edit_layout(&doc, 0, 0, text, &text::BlockStyle::default()).unwrap().draft_rect;
assert_eq!(layout("Hello there "), layout("Hello there"));
}
/// A lone carriage return (an old Mac line ending) breaks the row, as `\n` and `\r\n` do.
#[test]
fn a_lone_carriage_return_breaks_an_edited_paragraph() {
let mut doc = text_page("BT /F1 12 Tf 72 700 Td (Hello world) Tj ET");
text::replace_block(&mut doc, 0, 0, "one\rtwo").unwrap();
let lines: Vec<String> = text::text_lines(&reopen(&doc), 0).unwrap().into_iter().map(|l| l.text).collect();
assert_eq!(lines, ["one", "two"]);
}
/// On the styled rewrite path too, `\r\n` is one row break and a lone `\r` another, and no
/// carriage return reaches the page: with no other formatting, and with a part set in red.
#[test]
fn carriage_returns_break_rows_on_the_styled_path() {
let draft = "one\r\ntwo\rthree";
let red = text::RangeStyle {
range: 0..3,
family: None,
resource: None,
size: None,
color: Some([1.0, 0.0, 0.0]),
underline: None,
char_spacing: None,
scale: None,
};
for style in [text::BlockStyle::default(), text::BlockStyle { ranges: vec![red], ..Default::default() }] {
let mut doc = text_page("BT /F1 12 Tf 72 700 Td (Hello world) Tj ET");
text::rewrite_block(&mut doc, 0, 0, Some(draft), &style).unwrap();
let doc = reopen(&doc);
let content = page_content_bytes(&doc, 0);
assert!(!content.contains(&b'\r') && !content.windows(2).any(|w| w == b"\\r"), "{}", String::from_utf8_lossy(&content));
let lines: Vec<String> = text::text_lines(&doc, 0).unwrap().into_iter().map(|l| l.text).collect();
assert_eq!(lines, ["one", "two", "three"], "{style:?}");
}
}
/// A subset with no space glyph shows each space as a positioning gap one space wide, so a run
/// of spaces still takes a run's width (#4199).
#[test]
fn a_run_of_spaces_in_a_subset_without_a_space_glyph_keeps_its_width() {
let mut doc = text_page("BT /F2 12 Tf 72 700 Td (ab) Tj /F1 12 Tf ( ) Tj /F2 12 Tf (ba) Tj ET");
assert_eq!(text::replace_block(&mut doc, 0, 0, "ab ba ab").unwrap().substituted, None);
let content = String::from_utf8_lossy(&page_content_bytes(&reopen(&doc), 0)).into_owned();
assert_eq!(content.matches("[-330] TJ").count(), 5, "{content}");
}
+89 -27
View File
@@ -208,7 +208,17 @@ fn layout_draft(
*count -= 1;
continue;
}
rect = Some(rect.map_or(line.rect, |r| [r[0].min(line.rect[0]), r[1].min(line.rect[1]), r[2].max(line.rect[2]), r[3].max(line.rect[3])]));
// Spaces typed after a row's last word are drawn but show nothing, so an upright row is
// measured to the end of its last word: they don't make a draft overflow (#4199).
let mut r = line.rect;
if line.upright() && !line.ink.is_empty() {
if crate::bidi::rtl_base(&line.text) {
r[0] = r[0].max(line.ink.iter().map(|i| i[0]).fold(f64::INFINITY, f64::min));
} else {
r[2] = r[2].min(line.ink.iter().map(|i| i[2]).fold(f64::NEG_INFINITY, f64::max));
}
}
rect = Some(rect.map_or(r, |a| [a[0].min(r[0]), a[1].min(r[1]), a[2].max(r[2]), a[3].max(r[3])]));
}
let overflow = rect
.is_some_and(|r| r[0] < source.rect[0] - 0.5 || r[1] < source.rect[1] - 0.5 || r[2] > source.rect[2] + 0.5 || r[3] > source.rect[3] + 0.5);
@@ -2745,20 +2755,28 @@ fn paragraph_separator(c: char) -> bool {
/// Greedy word wrapping to `width` with `advance` giving a string's width. Each `\n` starts a
/// new line. A row may also end inside a word of Chinese or Japanese text ([`crate::linebreak`]);
/// those pieces join without a space.
/// those pieces join without a space. Every white-space character is written as one space, so
/// a run of spaces typed between, before or after words stays (#4199); only the spaces where a
/// row wraps are dropped.
fn wrap(text: &str, width: f64, advance: impl Fn(&str) -> f64) -> Vec<String> {
let mut out = Vec::new();
for row in text.split('\n') {
let mut line = String::new();
let start = out.len();
for word in row.split(paragraph_separator).filter(|word| !word.is_empty()) {
// The separators read since the last word: each one boundary between `split` pieces.
let mut gap = 0usize;
for (k, word) in row.split(paragraph_separator).enumerate() {
if k > 0 {
gap = gap.saturating_add(1);
}
if word.is_empty() {
continue;
}
let spaces = " ".repeat(std::mem::take(&mut gap));
for (i, piece) in crate::linebreak::pieces(word).into_iter().enumerate() {
let candidate = match (line.is_empty(), i) {
(true, _) => piece.to_string(),
(false, 0) => format!("{line} {piece}"),
(false, _) => format!("{line}{piece}"),
};
if !line.is_empty() && advance(&candidate) > width {
let candidate = if i == 0 { format!("{line}{spaces}{piece}") } else { format!("{line}{piece}") };
// A row holding only the spaces that indent it doesn't wrap before its first word.
if line.chars().any(|c| c != ' ') && advance(&candidate) > width {
out.push(std::mem::take(&mut line));
line = piece.to_string();
} else {
@@ -2766,6 +2784,7 @@ fn wrap(text: &str, width: f64, advance: impl Fn(&str) -> f64) -> Vec<String> {
}
}
}
line.push_str(&" ".repeat(gap));
if !line.is_empty() || out.len() == start {
out.push(line);
}
@@ -2887,7 +2906,8 @@ fn rewrite_block_from(
&& (!style.ranges.is_empty()
|| style.resource.is_some()
|| (text.is_some_and(|draft| {
draft.contains('\n')
// A row break, `\n` or a carriage return, takes the path that keeps untouched rows.
draft.contains(['\n', '\r'])
|| pdfcraft_fonts::source_shaping::required(draft)
|| !pdfcraft_fonts::embedding::matching_faces(&b.base_font).is_empty()
}) || registered_face
@@ -2902,13 +2922,19 @@ fn rewrite_block_from(
let streams = content_streams(doc, &p.dict)?;
let pieces = Pieces::join(&streams.iter().map(|(_, d)| d.as_slice()).collect::<Vec<_>>());
let ops = pieces.parse().ops;
// Explicit line breaks stay (#1443); within a line, runs of white space become one space.
let text = text
// Explicit line breaks stay (#1443), and so does each typed U+0020 space: a run of them is
// not folded into one (#4199, #3797). `\r\n` and a lone `\r` are breaks too; other white
// space is written as one space.
let text: String = text
.unwrap_or(&b.text)
.split('\n')
.map(|row| row.split(paragraph_separator).filter(|word| !word.is_empty()).collect::<Vec<_>>().join(" "))
.collect::<Vec<_>>()
.join("\n");
.replace("\r\n", "\n")
.chars()
.map(|c| match c {
'\r' => '\n',
c if c != '\n' && paragraph_separator(c) => ' ',
c => c,
})
.collect();
// The characters to show, for choosing fonts: the breaks are moves, not glyphs.
let flat = text.replace('\n', " ");
let o = &first.origin;
@@ -3124,12 +3150,13 @@ fn rewrite_block_from(
block_ops.extend(state.ops());
block_ops.push(Op::new("Tm", tm.iter().map(|v| n(*v)).collect()));
// Alignment: each line's offset from the left edge, in text space.
let offset = |line: &str| -> f64 {
let free = (width - advance(line)).max(0.0) / k;
// `visible` is the width the line is aligned on (see the loop below).
let offset = |visible: f64| -> f64 {
let free = (width - visible).max(0.0) / k;
match align {
Some(crate::added::Align::Center) => free / 2.0,
// A right-to-left line anchored at its right edge may start left of the old box.
Some(crate::added::Align::Right) if anchored_right => (right_edge - advance(line)) / k,
Some(crate::added::Align::Right) if anchored_right => (right_edge - visible) / k,
Some(crate::added::Align::Right) => free,
// A justified right-to-left paragraph ends its last line at the right edge.
Some(crate::added::Align::Justify) if rtl => free,
@@ -3149,11 +3176,29 @@ fn rewrite_block_from(
let mut tw_set = 0.0;
let mut underlines: Vec<(f64, f64, f64)> = Vec::new(); // (x0, x1, y) in text space
for (i, line) in wrapped.iter().enumerate() {
let spaces = line.matches(' ').count();
let tw =
if justify && i + 1 < wrapped.len() && spaces > 0 { (width - advance(line)).max(0.0) / k / o_state.scale / spaces as f64 } else { 0.0 };
// A stretched line fills the width from the left edge.
let dx = if tw > 0.0 { 0.0 } else { offset(line) };
// A stretched (justified) line shows only its words, as before #4199: word spacing would
// reach the spaces before its first word and after its last, so they are left out, and
// the line fills the width from the left edge. Nothing to stretch over, or a paragraph
// drawn at `0 Tz`, isn't justified.
let body = line.trim_matches(' ');
let interior = body.matches(' ').count();
let tw = if justify && i + 1 < wrapped.len() && interior > 0 && o_state.scale.abs() > 1e-6 {
Some((width - advance(body)).max(0.0) / k / o_state.scale / interior as f64).filter(|tw| tw.is_finite())
} else {
None
}
.unwrap_or(0.0);
// Other lines show every space typed. A left-to-right one is aligned on its width up to
// its last word; a right-to-left one on its whole width, since the spaces typed after its
// last word are drawn on its left and the ones before its first on its right.
let (shown, visible) = if tw > 0.0 {
(body, advance(body))
} else if crate::bidi::rtl_base(line) {
(line.as_str(), advance(line))
} else {
(line.as_str(), advance(line.trim_end_matches(' ')))
};
let dx = if tw > 0.0 { 0.0 } else { offset(visible) };
if i > 0 || dx != 0.0 {
block_ops.push(Op::new("Td", vec![n(dx - x), n(if i > 0 { -lead } else { 0.0 })]));
}
@@ -3165,7 +3210,7 @@ fn rewrite_block_from(
tw_set = tw;
}
// Each wrapped line in typing order; drawn left to right (right-to-left runs reversed).
let visual = crate::bidi::to_visual(line);
let visual = crate::bidi::to_visual(shown);
// Stretched without word spacing: shown up to and including each space, then moved on.
let pieces = if stretch > 0.0 { visual.split_inclusive(' ').collect() } else { vec![visual.as_str()] };
for piece in pieces {
@@ -3186,7 +3231,8 @@ fn rewrite_block_from(
}
}
let tw = tw + stretch;
let w = (advance(line) + tw * spaces as f64 * o_state.scale * k) / k;
// Underlined to the line's last word, over its stretched gaps.
let w = (visible + tw * interior as f64 * o_state.scale * k) / k;
underlines.push((dx, dx + w, -(i as f64) * lead - size * 0.12));
}
block_ops.push(Op::new("ET", vec![]));
@@ -3537,7 +3583,23 @@ mod tests {
assert_eq!(wrap("A  B C", 4.0, width), ["A  B", "C"]);
assert_eq!(wrap("A\u{a0}B C", 3.0, width), ["A B", "C"]);
assert_eq!(wrap("A\u{2028}B C", 3.0, width), ["A B", "C"]);
assert_eq!(wrap(" \tA B\r\nC ", 3.0, width), ["A B", "C"]);
}
/// #4199: a run of spaces stays a run, before, between and after words; the spaces where a
/// row wraps go, and each other white-space character is one space.
#[test]
fn paragraph_wrapping_keeps_every_space_typed() {
let width = |s: &str| s.chars().count() as f64;
assert_eq!(wrap("a b c", 20.0, width), ["a b c"]);
assert_eq!(wrap(" a b ", 20.0, width), [" a b "]);
assert_eq!(wrap("a\t\tb", 20.0, width), ["a b"]);
assert_eq!(wrap("aa bb cc", 6.0, width), ["aa bb", "cc"]);
// An indent alone on a row doesn't wrap before its first word.
assert_eq!(wrap(" abc", 3.0, width), [" abc"]);
// The rewrite turns `\r\n` and `\r` into `\n` before wrapping.
assert_eq!(wrap(" \tA B\nC ", 3.0, width), [" A", "B", "C "]);
assert_eq!(wrap(" ", 3.0, width), [" "]);
assert_eq!(wrap("", 3.0, width), [""]);
}
#[test]
+15 -1
View File
@@ -547,11 +547,20 @@ pub(super) fn rewrite_styled_block(
let mut programs: HashMap<String, Option<std::sync::Arc<Vec<u8>>>> = HashMap::new();
for (range, index) in styled_spans {
let source = lines.get(index).unwrap_or(first);
for ch in draft.get(range).unwrap_or_default().chars() {
let start = range.start;
for (offset, ch) in draft.get(range).unwrap_or_default().char_indices() {
if ch == '\n' {
chars.push((ch, index, source.font.clone()));
continue;
}
// A carriage return breaks the row too. Before `\n` the pair is one break, so the
// CR stays a `\r` the rows skip. Either way it keeps its place among the draft's
// characters, which rows are counted in.
if ch == '\r' {
let crlf = start.checked_add(offset).and_then(|at| draft.get(at.saturating_add(1)..)).is_some_and(|rest| rest.starts_with('\n'));
chars.push((if crlf { '\r' } else { '\n' }, index, source.font.clone()));
continue;
}
let s = ch.to_string();
let name = if let Some(resource) = overrides.get(&index).and_then(|r| r.resource.as_ref()).or(style.resource.as_ref()) {
if !metrics.get(resource).is_some_and(|m| source_encodes(m, &s)) {
@@ -735,6 +744,11 @@ pub(super) fn rewrite_styled_block(
let mut at = 0;
while at < chars.len() {
let Some(item) = chars.get(at) else { break };
// The CR of a CRLF: the `\n` after it breaks the row.
if item.0 == '\r' {
at += 1;
continue;
}
if item.0 == '\n' {
row_ranges.push(row_start..row_start + row.len());
rows.push(std::mem::take(&mut row));
+12
View File
@@ -6930,6 +6930,18 @@ fn text_blocks_stay_cached_after_an_edit_and_follow_undo_and_redo() {
assert!(Arc::ptr_eq(&redone, &blocks(&session)));
}
/// #4199, #3797: the spaces typed into a paragraph are all there after saving and reopening.
#[test]
fn edited_text_keeps_every_space_typed_through_save_and_reopen() {
let (mut s, id) = session_with(1);
let typed = " A b c";
s.apply(id, Edit::EditTextBlock { page: 0, block: 0, text: typed.into(), style: pdfcraft_edit::BlockStyle::default() }).unwrap();
let saved = s.save_bytes(id).unwrap();
let reopened = s.open("saved.pdf", None, saved, None).unwrap();
let blocks = s.get(reopened).unwrap().editor.as_ref().unwrap().text_blocks(0).unwrap();
assert_eq!(blocks[0].editor_text, typed);
}
/// A new document, or one restored from recovery, has never been written where it belongs: undo
/// back to the bytes it was opened from must not make it clean, or closing it would discard the
/// whole document without asking. Saving makes it clean; Revert of a saved one too.