From 9e391e97f42eba5b40a6370ce7be9e1d2caa64d8 Mon Sep 17 00:00:00 2001 From: vangel76 Date: Wed, 4 Nov 2026 02:04:56 +0100 Subject: [PATCH] Paragraph tools: skip tracked-deleted characters; snapshot revisions once per command (review) Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/fmt_revisions.rs | 30 ++++++++---- crates/engine/src/cmd/para/tools.rs | 63 ++++++++++++++++---------- crates/engine/src/tests/para_tools.rs | 47 +++++++++++++++++++ 3 files changed, 109 insertions(+), 31 deletions(-) diff --git a/crates/engine/src/cmd/fmt_revisions.rs b/crates/engine/src/cmd/fmt_revisions.rs index 3d7e23edd8..a13b9a6251 100644 --- a/crates/engine/src/cmd/fmt_revisions.rs +++ b/crates/engine/src/cmd/fmt_revisions.rs @@ -212,16 +212,30 @@ pub(crate) fn record(s: &mut Session, id: &str, before: &Document) { } } -/// Build the tracked paragraph-property change for a command that also changes the paragraph's +/// Builds the tracked paragraph-property change for a command that also changes the paragraph's /// text. The ordinary after-command recorder deliberately compares only paragraphs with identical -/// text, so mixed text/paragraph-format transformations call this before assigning their props. -pub(crate) fn changed_para_props(s: &mut Session, before: &ParaProps, after: &ParaProps) -> Option>> { - if !s.doc.settings.track_changes || !s.prefs.markup.track_formatting { - return after.fmt_change.clone(); +/// text, so mixed text/paragraph-format transformations use this before assigning their props. +/// Made once per command, before it creates revisions: it keeps the revisions as they were then +/// (all `Recorder::own` needs), instead of copying the list for every paragraph. +pub(crate) struct ParaPropTracker { + before: Vec, +} + +impl ParaPropTracker { + pub(crate) fn new(s: &Session) -> Self { + let tracking = s.doc.settings.track_changes && s.prefs.markup.track_formatting; + ParaPropTracker { before: if tracking { s.doc.revisions.clone() } else { Vec::new() } } + } + + pub(crate) fn changed(&self, s: &mut Session, before: &ParaProps, after: &ParaProps) -> Option>> { + if !s.doc.settings.track_changes || !s.prefs.markup.track_formatting { + return after.fmt_change.clone(); + } + let mut r = Recorder { author: s.author.clone(), date: super::now_iso(), before: &self.before, revisions: &mut s.doc.revisions, rid: None }; + r.change(before, after, ParaProps::formatting, |p| &p.fmt_change) + .map(|Replacement(change)| change) + .unwrap_or_else(|| after.fmt_change.clone()) } - let revisions_before = s.doc.revisions.clone(); - let mut r = Recorder { author: s.author.clone(), date: super::now_iso(), before: &revisions_before, revisions: &mut s.doc.revisions, rid: None }; - r.change(before, after, ParaProps::formatting, |p| &p.fmt_change).map(|Replacement(change)| change).unwrap_or_else(|| after.fmt_change.clone()) } /// Compare Documents: record how paragraph `new` (the revised version, same text) is formatted diff --git a/crates/engine/src/cmd/para/tools.rs b/crates/engine/src/cmd/para/tools.rs index 63160bfe10..2eba2cbec3 100644 --- a/crates/engine/src/cmd/para/tools.rs +++ b/crates/engine/src/cmd/para/tools.rs @@ -96,9 +96,16 @@ fn apply_deletions(s: &mut Session, story: StoryRef, path: &Path, mut ranges: Ve Ok(()) } -/// The byte ranges of every manual line break in the paragraph. +/// The characters of the paragraph that are not tracked-deleted, as (byte offset, char). Under +/// Track Changes deleted text stays in `para.text`, so every scan that decides what to delete or +/// count goes through this. +fn live_chars(para: &wordcraft_doc::Paragraph) -> impl Iterator + '_ { + para.text.char_indices().filter(|(i, _)| para.props_of_char(*i).del.is_none()) +} + +/// The byte ranges of every manual line break in the paragraph that is not already deleted. fn break_ranges(para: &wordcraft_doc::Paragraph) -> Vec<(usize, usize)> { - para.text.char_indices().filter(|(_, c)| *c == LINE_BREAK).map(|(i, _)| (i, i + 1)).collect() + live_chars(para).filter(|(_, c)| *c == LINE_BREAK).map(|(i, _)| (i, i + 1)).collect() } /// `para.convertBreaks`: every manual line break (Shift+Enter) in the selection becomes a @@ -107,7 +114,7 @@ pub(super) fn convert_breaks(s: &mut Session, v: &Value) -> CmdResult { scope_all(s, v)?; let (story, paths) = targets(s)?; check_target_edits(s, story, &paths)?; - let breaks: usize = paths.iter().filter_map(|path| s.doc.para(story, path)).map(|p| p.text.chars().filter(|c| *c == LINE_BREAK).count()).sum(); + let breaks: usize = paths.iter().filter_map(|path| s.doc.para(story, path)).map(|p| break_ranges(p).len()).sum(); if breaks > MAX_TOOL_PARAS { return Err(CmdError::Params(format!("convert at most {MAX_TOOL_PARAS} line breaks at once"))); } @@ -178,11 +185,17 @@ pub(super) fn remove_blank(s: &mut Session, v: &Value) -> CmdResult { sel_result(s) } -/// The number of leading spaces in the paragraph's text. +/// The number of leading spaces in the paragraph's text, deleted ones included (the range +/// `trimLeadingSpaces` deletes). fn leading_spaces(para: &wordcraft_doc::Paragraph) -> usize { para.text.chars().take_while(|c| *c == ' ').count() } +/// The byte offsets of the paragraph's leading spaces that are not tracked-deleted. +fn live_leading_spaces(para: &wordcraft_doc::Paragraph) -> Vec { + live_chars(para).take_while(|(_, c)| *c == ' ').map(|(i, _)| i).collect() +} + /// `para.trimLeadingSpaces`: delete the spaces at the start of each selected paragraph. pub(super) fn trim_leading(s: &mut Session, v: &Value) -> CmdResult { scope_all(s, v)?; @@ -205,24 +218,25 @@ pub(super) fn remove_extra_spaces(s: &mut Session, v: &Value) -> CmdResult { check_target_edits(s, story, &paths)?; for path in paths { let Some(para) = s.doc.para(story, &path) else { continue }; + // Runs of live spaces (deleted characters are skipped, as if absent). A run at the + // paragraph's start stays; a run inside the text collapses to its last space, and one + // that reaches the paragraph's end to its first. let mut runs = Vec::new(); - let mut start = None; - for (i, c) in para.text.char_indices() { + let mut run: Vec = Vec::new(); + let mut leading = true; + for (i, c) in live_chars(para) { if c == ' ' { - start.get_or_insert(i); - } else if let Some(a) = start.take() { - // A run at the paragraph's start stays; a run inside the text collapses to one space. - if a > 0 && i - a > 1 { - runs.push((a, i - 1)); - } + run.push(i); + continue; } + if !leading && run.len() > 1 { + runs.extend(run[..run.len() - 1].iter().map(|&a| (a, a + 1))); + } + run.clear(); + leading = false; } - // A run that reaches the paragraph's end collapses to one space too. - if let Some(a) = start - && a > 0 - && para.len() - a > 1 - { - runs.push((a + 1, para.len())); + if !leading && run.len() > 1 { + runs.extend(run[1..].iter().map(|&a| (a, a + 1))); } apply_deletions(s, story, &path, runs)?; } @@ -235,9 +249,11 @@ pub(super) fn spaces_to_indent(s: &mut Session, v: &Value) -> CmdResult { scope_all(s, v)?; let (story, paths) = targets(s)?; check_target_edits(s, story, &paths)?; + let tracker = crate::cmd::fmt_revisions::ParaPropTracker::new(s); for path in paths { let Some(para) = s.doc.para(story, &path) else { continue }; - let spaces = leading_spaces(para); + let live = live_leading_spaces(para); + let spaces = live.len(); if spaces == 0 { continue; } @@ -250,14 +266,14 @@ pub(super) fn spaces_to_indent(s: &mut Session, v: &Value) -> CmdResult { let mut after = before.clone(); after.indent_first = Some(current + represented as f32 * 72.0); after.cancel_char_indents(false, false, true); - after.fmt_change = crate::cmd::fmt_revisions::changed_para_props(s, &before, &after); + after.fmt_change = tracker.changed(s, &before, &after); { let p = s.doc.para_mut(story, &path)?; p.props = after; p.touch(); } - // A space is one byte, so the byte range is the count. - apply_deletions(s, story, &path, vec![(0, represented)])?; + // A space is one byte; deleted spaces in between stay as they are. + apply_deletions(s, story, &path, live[..represented].iter().map(|&a| (a, a + 1)).collect())?; } sel_result(s) } @@ -268,6 +284,7 @@ pub(super) fn indent_to_spaces(s: &mut Session, v: &Value) -> CmdResult { scope_all(s, v)?; let (story, paths) = targets(s)?; check_target_edits(s, story, &paths)?; + let tracker = crate::cmd::fmt_revisions::ParaPropTracker::new(s); for path in paths { let Some(para) = s.doc.para(story, &path) else { continue }; let pts = para.props.indent_left.unwrap_or(0.0) + para.props.indent_first.unwrap_or(0.0); @@ -290,7 +307,7 @@ pub(super) fn indent_to_spaces(s: &mut Session, v: &Value) -> CmdResult { after.indent_first = Some(0.0); after.indent_left = Some(0.0); after.cancel_char_indents(true, false, true); - after.fmt_change = crate::cmd::fmt_revisions::changed_para_props(s, &before, &after); + after.fmt_change = tracker.changed(s, &before, &after); let p = s.doc.para_mut(story, &path)?; if n > 0 { p.insert_text(0, &" ".repeat(n), &props)?; diff --git a/crates/engine/src/tests/para_tools.rs b/crates/engine/src/tests/para_tools.rs index fedd76c870..fc782ae3ab 100644 --- a/crates/engine/src/tests/para_tools.rs +++ b/crates/engine/src/tests/para_tools.rs @@ -400,6 +400,53 @@ fn deletions_are_recorded_under_track_changes() { assert_eq!(text(&s), "a b\nc", "accepting removes the leading space"); } +/// Tracked-delete the bytes `a..b` of the first body paragraph (Track Changes must be on). +fn track_delete_in_first(s: &mut Session, a: usize, b: usize) { + let path = s.doc.para_paths(StoryRef::Body)[0].clone(); + let at = |off: usize| Pos { story: StoryRef::Body, path: path.clone(), off }; + crate::cmd::track_delete(s, &at(a), &at(b)).expect("tracked delete"); +} + +fn first_indent(s: &Session) -> Option { + let path = s.doc.para_paths(StoryRef::Body)[0].clone(); + s.doc.para(StoryRef::Body, &path).expect("paragraph").props.indent_first +} + +#[test] +fn remove_extra_spaces_skips_tracked_deleted_spaces() { + let mut s = s(); + run(&mut s, "document.setText", json!({"text": "a b"})); + run(&mut s, "review.trackChanges", json!({"value": true})); + track_delete_in_first(&mut s, 2, 3); // the second space is already deleted + select_all(&mut s); + run(&mut s, "para.removeExtraSpaces", json!({"scope": "all"})); + run(&mut s, "review.acceptAll", json!({})); + assert_eq!(text(&s), "a b", "the one live space survives"); +} + +#[test] +fn spaces_to_indent_twice_does_not_count_deleted_spaces_again() { + let mut s = s(); + run(&mut s, "document.setText", json!({"text": " x"})); + run(&mut s, "review.trackChanges", json!({"value": true})); + select_all(&mut s); + run(&mut s, "para.spacesToIndent", json!({"scope": "all"})); + assert_eq!(first_indent(&s), Some(144.0)); + run(&mut s, "para.spacesToIndent", json!({"scope": "all"})); + assert_eq!(first_indent(&s), Some(144.0), "the deleted spaces are not indented a second time"); +} + +#[test] +fn convert_breaks_leaves_a_tracked_deleted_break_alone() { + let mut s = s(); + type_breaks(&mut s, "a\nb"); + run(&mut s, "review.trackChanges", json!({"value": true})); + track_delete_in_first(&mut s, 1, 2); + select_all(&mut s); + run(&mut s, "para.convertBreaks", json!({"scope": "all"})); + assert_eq!(s.doc.para_paths(StoryRef::Body).len(), 1, "a deleted break is not turned into a paragraph break"); +} + #[test] fn junk_scope_is_refused() { let mut s = s();