diff --git a/crates/engine/src/cmd/review.rs b/crates/engine/src/cmd/review.rs index 516ddd3dc..6ce7a6e3c 100644 --- a/crates/engine/src/cmd/review.rs +++ b/crates/engine/src/cmd/review.rs @@ -363,20 +363,8 @@ fn nav_comment(s: &mut Session, dir: i32) -> CmdResult { sel_result(s) } -/// Accept or reject a revision range in one paragraph; `mark` includes its paragraph mark (and -/// the section break on it), `props` the paragraph's own formatting change. -#[allow(clippy::too_many_arguments)] -fn resolve_para( - s: &mut Session, - story: StoryRef, - path: &wordcraft_doc::Path, - from: usize, - to: usize, - mark: bool, - props: bool, - accept: bool, -) -> Result<(), CmdError> { - let para = s.doc.para_mut(story, path)?; +/// Accept or reject the tracked formatting and text of `from..to` in one paragraph (not its mark). +fn resolve_runs(para: &mut wordcraft_doc::Paragraph, from: usize, to: usize, props: bool, accept: bool) -> Result<(), CmdError> { if props && (para.props.fmt_change.is_some() || para.props.num_change.is_some()) { fmt_revisions::resolve_para(&mut para.props, accept); para.touch(); @@ -397,6 +385,24 @@ fn resolve_para( })?; } } + Ok(()) +} + +/// Accept or reject a revision range in one paragraph; `mark` includes its paragraph mark (and +/// the section break on it), `props` the paragraph's own formatting change. +#[allow(clippy::too_many_arguments)] +fn resolve_para( + s: &mut Session, + story: StoryRef, + path: &wordcraft_doc::Path, + from: usize, + to: usize, + mark: bool, + props: bool, + accept: bool, +) -> Result<(), CmdError> { + let para = s.doc.para_mut(story, path)?; + resolve_runs(para, from, to, props, accept)?; if !mark { return Ok(()); } @@ -513,6 +519,28 @@ pub fn move_changes(s: &Session) -> Vec<(Pos, String, bool)> { out } +/// Accept or reject the tracked text and paragraph marks of the top-level paragraphs in a block +/// list that is not a story (a note separator). +fn resolve_loose_blocks(blocks: &mut wordcraft_doc::Blocks, accept: bool) -> Result<(), CmdError> { + for b in blocks.iter_mut() { + let wordcraft_doc::Block::Para(p) = std::sync::Arc::make_mut(b) else { continue }; + let len = p.len(); + resolve_runs(p, 0, len, true, accept)?; + if p.mark.fmt_change.is_some() || p.section.as_ref().is_some_and(|x| x.fmt_change.is_some()) { + fmt_revisions::resolve_char(&mut p.mark, accept); + if let Some(sec) = p.section.as_deref_mut() { + fmt_revisions::resolve_section(sec, accept); + } + } + // A mark can't be removed here (no neighbour to join), so it just stops being a revision. + p.mark.ins = None; + p.mark.del = None; + p.touch(); + } + fmt_revisions::resolve_containers(blocks, accept, 0); + Ok(()) +} + fn resolve_all(s: &mut Session, accept: bool) -> CmdResult { let stories: Vec = std::iter::once(StoryRef::Body).chain(s.doc.parts.keys().map(|k| StoryRef::Part(*k))).collect(); for st in stories { @@ -527,6 +555,13 @@ fn resolve_all(s: &mut Session, accept: bool) -> CmdResult { fmt_revisions::resolve_containers(&mut part.blocks, accept, 0); } fmt_revisions::resolve_section(&mut s.doc.last_section, accept); + // The note separators are block lists of their own, not stories; they must not keep ids of + // revisions that are about to be dropped. + for sep in [&mut s.doc.footnote_separators, &mut s.doc.endnote_separators] { + for blocks in [&mut sep.separator, &mut sep.continuation_separator, &mut sep.continuation_notice].into_iter().flatten() { + resolve_loose_blocks(blocks, accept)?; + } + } s.doc.revisions.clear(); s.clamp_selection(); sel_result(s) diff --git a/crates/engine/src/tests_revision_checks.rs b/crates/engine/src/tests_revision_checks.rs index 71ce49ab1..fe3e61603 100644 --- a/crates/engine/src/tests_revision_checks.rs +++ b/crates/engine/src/tests_revision_checks.rs @@ -401,3 +401,74 @@ fn tracked_delete_of_struck_table_text_keeps_it_for_reject_all() { assert_eq!(cells(&s), [["A1", "B1"], ["A2", "B2"]], "{id}"); } } + +#[derive(Clone, Copy, Debug)] +enum Slot { + FootnoteSeparator, + EndnoteSeparator, + FootnoteContinuationSeparator, + EndnoteContinuationNotice, +} + +const SLOTS: [Slot; 4] = [Slot::FootnoteSeparator, Slot::EndnoteSeparator, Slot::FootnoteContinuationSeparator, Slot::EndnoteContinuationNotice]; + +fn slot_mut(doc: &mut wordcraft_doc::Document, slot: Slot) -> &mut Option { + match slot { + Slot::FootnoteSeparator => &mut doc.footnote_separators.separator, + Slot::EndnoteSeparator => &mut doc.endnote_separators.separator, + Slot::FootnoteContinuationSeparator => &mut doc.footnote_separators.continuation_separator, + Slot::EndnoteContinuationNotice => &mut doc.endnote_separators.continuation_notice, + } +} + +/// A session whose note separator `slot` holds the text "SEP" as a tracked insertion (revision 0). +fn tracked_separator_session(slot: Slot) -> Session { + let mut doc = wordcraft_doc::Document::new(); + doc.revisions.push(wordcraft_doc::Revision { + kind: wordcraft_doc::RevisionKind::Insert, + author: "Alice".into(), + date: String::new(), + move_name: None, + }); + let props = wordcraft_doc::CharProps { ins: Some(0), ..Default::default() }; + let p = wordcraft_doc::Paragraph::with_text("SEP", props); + *slot_mut(&mut doc, slot) = Some(vec![wordcraft_doc::para_block(p)]); + Session::new(doc) +} + +fn separator_para(s: &mut Session, slot: Slot) -> wordcraft_doc::Paragraph { + slot_mut(&mut s.doc, slot).as_ref().and_then(|b| b.iter().find_map(|b| b.as_para())).expect("separator").clone() +} + +#[test] +fn accept_all_resolves_note_separator_revisions() { + for slot in SLOTS { + let mut s = tracked_separator_session(slot); + run(&mut s, "review.acceptAll", json!({})); + let p = separator_para(&mut s, slot); + assert_eq!(p.plain_text(), "SEP", "{slot:?}"); + assert!(p.run_ranges().all(|(_, c)| c.ins.is_none() && c.del.is_none()), "{slot:?}"); + } +} + +#[test] +fn reject_all_resolves_note_separator_revisions() { + for slot in SLOTS { + let mut s = tracked_separator_session(slot); + run(&mut s, "review.rejectAll", json!({})); + assert_eq!(separator_para(&mut s, slot).plain_text(), "", "{slot:?}"); + } +} + +#[test] +fn new_tracked_change_does_not_alias_separator_revision() { + for slot in SLOTS { + let mut s = tracked_separator_session(slot); + run(&mut s, "review.acceptAll", json!({})); + s.author = "Bob".into(); + run(&mut s, "review.trackChanges", json!({"value": true})); + run(&mut s, "text.insert", json!({"text": "NEW "})); + let p = separator_para(&mut s, slot); + assert!(p.run_ranges().all(|(_, c)| c.ins.is_none() && c.del.is_none()), "{slot:?}"); + } +}