From 066e49bf80d733c8d0f64df7f96a2713b9f70d24 Mon Sep 17 00:00:00 2001 From: Boris Kudryashov Date: Mon, 26 Oct 2026 01:19:58 +0300 Subject: [PATCH 1/5] projcore: Task::rich_notes and Editor::set_rich_notes (#1801) A task's notes can carry richtext markup beside the plain text every reader uses. set_rich_notes splits a value into the two (markup kept only when it formats something), and a plain set_notes of new text drops the markup so old formatting never comes back over new text. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01L2XCadpiTmNZGJmLxXsMvK --- projcore/src/editor.rs | 32 +++++++- projcore/src/editor/cells/tests.rs | 126 +++++++++++++++++++++++++++++ projcore/src/model.rs | 5 ++ projcore/src/richtext.rs | 53 ++++++++++++ 4 files changed, 215 insertions(+), 1 deletion(-) diff --git a/projcore/src/editor.rs b/projcore/src/editor.rs index 5f54ca6d6..beb1817bb 100644 --- a/projcore/src/editor.rs +++ b/projcore/src/editor.rs @@ -1573,7 +1573,37 @@ impl Editor { return Ok(false); } self.snapshot(); - self.proj.tasks[i].notes = notes; + let task = &mut self.proj.tasks[i]; + task.notes = notes; + // New plain text: the old formatting does not describe it (#1801). + task.rich_notes = None; + self.changed(); + Ok(true) + } + + /// Set a task's notes from a [`crate::richtext`] value as ONE undo step + /// (#1801): `notes` becomes its plain text (newline-normalised, empty + /// removes the notes) and `rich_notes` its canonical markup, kept only + /// when it formats something (`richtext::split_notes`). A value that is + /// not markup is plain text. Equal plain text with other formatting is a + /// change; the value a task already has changes nothing (`Ok(false)`). A + /// blank row cannot take notes. + pub fn set_rich_notes(&mut self, uid: i32, value: &str) -> Result { + let i = self.index(uid)?; + if self.proj.tasks[i].is_null { + return Err("A blank row cannot take notes".into()); + } + let (notes, rich) = crate::richtext::split_notes(value); + let task = &self.proj.tasks[i]; + if task.notes.as_deref().unwrap_or("") == notes.as_deref().unwrap_or("") + && task.rich_notes == rich + { + return Ok(false); + } + self.snapshot(); + let task = &mut self.proj.tasks[i]; + task.notes = notes; + task.rich_notes = rich; self.changed(); Ok(true) } diff --git a/projcore/src/editor/cells/tests.rs b/projcore/src/editor/cells/tests.rs index e3dcd529f..261d7eca3 100644 --- a/projcore/src/editor/cells/tests.rs +++ b/projcore/src/editor/cells/tests.rs @@ -246,6 +246,132 @@ fn set_notes_survive_mspdi_save() { assert!(!xml.contains("")); } +/// Criterion 1 of #1801: markup is kept only when it formats, and its +/// plain text is then the notes. +fn rich_notes_consistent(task: &crate::Task) -> bool { + match &task.rich_notes { + None => true, + Some(m) => crate::richtext::split_notes(m) == (task.notes.clone(), Some(m.clone())), + } +} + +const BOLD_NOTE: &str = "

Line 1

\n
  • Line 2
  • "; + +#[test] +fn rich_notes_set_plain_and_markup_as_one_step() { + let mut ed = editor(); + let before = ed.project().clone(); + assert!(ed.set_rich_notes(10, BOLD_NOTE).unwrap()); + assert_eq!(ed.undo_depth(), 1); + assert!(ed.dirty()); + let task = ed.project().task(10).unwrap(); + assert_eq!(task.notes.as_deref(), Some("Line 1\nLine 2")); + assert_eq!(task.rich_notes.as_deref(), Some(BOLD_NOTE)); + assert!(rich_notes_consistent(task)); + let after = ed.project().clone(); + assert!(ed.undo()); + assert_eq!(ed.project(), &before); + assert!(ed.redo()); + assert_eq!(ed.project(), &after); +} + +#[test] +fn rich_notes_style_only_change_is_a_step() { + let mut ed = editor(); + assert!(ed.set_notes(10, "Line 1\nLine 2").unwrap()); + ed.mark_saved(); + let depth = ed.undo_depth(); + // Same plain text, bold and a bullet added: a change of its own. + assert!(ed.set_rich_notes(10, BOLD_NOTE).unwrap()); + assert_eq!(ed.undo_depth(), depth + 1); + assert!(ed.dirty()); + assert!(ed.undo()); + let task = ed.project().task(10).unwrap(); + assert_eq!(task.notes.as_deref(), Some("Line 1\nLine 2")); + assert_eq!(task.rich_notes, None); +} + +#[test] +fn rich_notes_identical_is_no_step() { + let mut ed = editor(); + assert!(ed.set_rich_notes(10, BOLD_NOTE).unwrap()); + ed.mark_saved(); + let history = (ed.undo_depth(), ed.redo_depth(), ed.dirty()); + assert!(!ed.set_rich_notes(10, BOLD_NOTE).unwrap()); + // A non-canonical spelling of the same value is the same value. + assert!( + !ed.set_rich_notes(10, "

    Line 1

    \n
  • Line 2
  • ") + .unwrap() + ); + assert_eq!((ed.undo_depth(), ed.redo_depth(), ed.dirty()), history); + // Plain markup of the plain notes a task has is no change either. + assert!(ed.set_notes(10, "x").unwrap()); + let history = (ed.undo_depth(), ed.redo_depth(), ed.dirty()); + assert!(!ed.set_rich_notes(10, "

    x

    ").unwrap()); + assert!(!ed.set_rich_notes(10, "x").unwrap()); + assert_eq!((ed.undo_depth(), ed.redo_depth(), ed.dirty()), history); +} + +#[test] +fn rich_notes_without_formatting_store_none() { + let mut ed = editor(); + assert!(ed.set_rich_notes(10, "

    a & b

    \n

    ").unwrap()); + let task = ed.project().task(10).unwrap(); + assert_eq!(task.notes.as_deref(), Some("a & b\n")); + assert_eq!(task.rich_notes, None); + // Not markup: plain text, newline-normalised. + assert!(ed.set_rich_notes(10, "if ac\r\nd").unwrap()); + let task = ed.project().task(10).unwrap(); + assert_eq!(task.notes.as_deref(), Some("if ac\nd")); + assert_eq!(task.rich_notes, None); + // Empty removes the notes, formatting and all. + assert!(ed.set_rich_notes(10, BOLD_NOTE).unwrap()); + assert!(ed.set_rich_notes(10, "
  • ").unwrap()); + let task = ed.project().task(10).unwrap(); + assert_eq!((task.notes.clone(), task.rich_notes.clone()), (None, None)); + // A blank row refuses, as for plain notes. + let i = ed.insert_blank_row(Some(10)).unwrap(); + let blank = ed.project().tasks[i].uid; + assert_eq!( + ed.set_rich_notes(blank, BOLD_NOTE).unwrap_err(), + "A blank row cannot take notes" + ); +} + +#[test] +fn set_notes_new_text_clears_rich_notes() { + let mut ed = editor(); + ed.set_rich_notes(10, BOLD_NOTE).unwrap(); + let depth = ed.undo_depth(); + assert!(ed.set_notes(10, "Line 1\nLine 2!").unwrap()); + assert_eq!(ed.undo_depth(), depth + 1); + let task = ed.project().task(10).unwrap(); + assert_eq!(task.rich_notes, None); + assert!(rich_notes_consistent(task)); + // Undo brings both back as they were. + assert!(ed.undo()); + assert_eq!( + ed.project().task(10).unwrap().rich_notes.as_deref(), + Some(BOLD_NOTE) + ); + // Removing the notes drops the formatting too. + assert!(ed.set_notes(10, "").unwrap()); + assert_eq!(ed.project().task(10).unwrap().rich_notes, None); +} + +#[test] +fn set_notes_same_text_keeps_rich_notes() { + let mut ed = editor(); + ed.set_rich_notes(10, BOLD_NOTE).unwrap(); + let history = (ed.undo_depth(), ed.redo_depth(), ed.dirty()); + // A host rewriting the unchanged plain text loses nothing. + assert!(!ed.set_notes(10, "Line 1\r\nLine 2").unwrap()); + assert_eq!((ed.undo_depth(), ed.redo_depth(), ed.dirty()), history); + let task = ed.project().task(10).unwrap(); + assert_eq!(task.rich_notes.as_deref(), Some(BOLD_NOTE)); + assert!(rich_notes_consistent(task)); +} + #[test] fn resources_preserve_allocations_and_undo_creation_together() { let mut ed = editor(); diff --git a/projcore/src/model.rs b/projcore/src/model.rs index 800f1ccdd..5fea55e87 100644 --- a/projcore/src/model.rs +++ b/projcore/src/model.rs @@ -395,6 +395,11 @@ pub struct Task { pub hyperlink_sub_address: Option, pub ignore_resource_calendar: Option, pub notes: Option, + /// The notes' formatting as canonical [`crate::richtext`] markup (#1801): + /// `Some` only when it carries what `notes` cannot (bold, italic, a + /// non-left alignment or a bullet), and then its plain text is `notes`. + /// Saved in `formatting.xml` (``), never in MSPDI. + pub rich_notes: Option, pub earned_value_method: Option, pub recurring: Option, pub hide_bar: Option, diff --git a/projcore/src/richtext.rs b/projcore/src/richtext.rs index ddaede8ba..0823960f5 100644 --- a/projcore/src/richtext.rs +++ b/projcore/src/richtext.rs @@ -125,6 +125,25 @@ impl Rich { } } +/// A task's notes from a rich value (#1801): the plain text (`None` when +/// empty) and the canonical markup, `None` unless it carries formatting the +/// plain text cannot (bold, italic, a non-left alignment or a bullet). A +/// value that is not markup is plain text, newline-normalised; empty plain +/// text keeps no markup (an empty bullet is no note). +pub fn split_notes(value: &str) -> (Option, Option) { + if !is_markup(value) { + let plain = crate::text::normalize_newlines(value); + return ((!plain.is_empty()).then_some(plain), None); + } + let rich = parse_markup(value); + if rich.text.is_empty() { + return (None, None); + } + let markup = rich.to_markup(); + let formatted = markup != Rich::plain(&rich.text).to_markup(); + (Some(rich.text), formatted.then_some(markup)) +} + /// The canonical markup of `text` with its attributes. Short or missing /// attributes are read as plain / left, so a desynced state still writes. pub fn to_markup(text: &str, chars: &[u8], paras: &[Para]) -> String { @@ -504,4 +523,38 @@ mod tests { invariant(&Rich::parse(s)); } } + + #[test] + fn split_notes_keeps_markup_only_when_it_formats() { + // Plain text, newline-normalised, no markup. + assert_eq!(split_notes("a\r\nb"), (Some("a\nb".into()), None)); + assert_eq!(split_notes(""), (None, None)); + // Markup without formatting is its plain text. + assert_eq!( + split_notes("

    a\tb

    \n

    \n

    c

    "), + (Some(" a\tb \n\nc".into()), None) + ); + // A plain note that looks like a tag is escaped, so it formats nothing. + let looks = Rich::plain("

    hi").to_markup(); + assert_eq!(looks, "

    <p>hi

    "); + assert_eq!(split_notes(&looks), (Some("

    hi".into()), None)); + // Bold, a bullet or an alignment keep the canonical markup. + for m in [ + "

    x y

    ", + "
  • x
  • ", + "

    x

    ", + "

    a & <b>

    \n
  • c
  • ", + ] { + let (plain, rich) = split_notes(m); + assert_eq!(rich.as_deref(), Some(m), "{m}"); + assert_eq!(plain, Some(Rich::parse(m).to_plain())); + } + // Not canonical in, canonical out. + assert_eq!( + split_notes("

    xy

    ").1.as_deref(), + Some("

    xy

    ") + ); + // An empty formatted line is no note. + assert_eq!(split_notes("
  • "), (None, None)); + } } From 200ea61c122e276027f4fad641ae959880125f85 Mon Sep 17 00:00:00 2001 From: Boris Kudryashov Date: Mon, 26 Oct 2026 01:22:39 +0300 Subject: [PATCH 2/5] projcore: rich task notes in formatting.xml, .yppx and the plugin seam (#1801) The markup travels where the formatting does, as children of formatting.xml, so a .mpp plugin gets it through the existing _ex symbols and a .yppx save keeps it. A note is applied on read only to a task whose plain notes it still describes; a plan without rich notes writes the part exactly as before (or none). Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01L2XCadpiTmNZGJmLxXsMvK --- projcore/src/formats/plugin.rs | 7 +- projcore/src/formatting.rs | 2 + projcore/src/formatting/notes.rs | 127 +++++++++++++++++++++++ projcore/src/formatting/notes/tests.rs | 138 +++++++++++++++++++++++++ projcore/src/yppx.rs | 53 ++++++++-- projcore/tests/plugins.rs | 64 ++++++++++++ 6 files changed, 383 insertions(+), 8 deletions(-) create mode 100644 projcore/src/formatting/notes.rs create mode 100644 projcore/src/formatting/notes/tests.rs diff --git a/projcore/src/formats/plugin.rs b/projcore/src/formats/plugin.rs index 15e31eff9..462daf60c 100644 --- a/projcore/src/formats/plugin.rs +++ b/projcore/src/formats/plugin.rs @@ -629,6 +629,9 @@ impl ProjectFormat for PluginFormat { .map_err(|_| format!("{}: the formatting it returned is not UTF-8", plugin.name))?; project.formatting = Formatting::from_xml(&text) .map_err(|e| format!("{}: the formatting it returned: {e}", plugin.name))?; + // The tasks' rich notes travel in the same text (#1801). + crate::formatting::apply_notes(&mut project, &text) + .map_err(|e| format!("{}: the formatting it returned: {e}", plugin.name))?; } Ok(project) } @@ -641,7 +644,9 @@ impl ProjectFormat for PluginFormat { )); } let xml = write_mspdi(project); - let formatting = (!project.formatting.is_empty()).then(|| project.formatting.to_xml()); + // The formatting and the tasks' rich notes (#1801), NULL without + // either. + let formatting = crate::formatting::part_xml(project); self.plugin .write_file(&self.ext_c, &xml, formatting.as_deref(), source) } diff --git a/projcore/src/formatting.rs b/projcore/src/formatting.rs index e5844a6be..e27ccd989 100644 --- a/projcore/src/formatting.rs +++ b/projcore/src/formatting.rs @@ -15,9 +15,11 @@ use opccore::xml::{Event, XmlParser}; pub mod bars; pub mod gridlines; pub mod layout; +pub mod notes; pub use bars::{BarRow, BarText, Facts, Mark, Middle, TaskBar}; pub use gridlines::{Gridline, Gridlines, LineType}; pub use layout::Layout; +pub use notes::{apply_notes, part_xml, rich_notes_json}; /// The `.yppx` part the formatting is saved in. pub const PART: &str = "formatting.xml"; diff --git a/projcore/src/formatting/notes.rs b/projcore/src/formatting/notes.rs new file mode 100644 index 000000000..9d37c5789 --- /dev/null +++ b/projcore/src/formatting/notes.rs @@ -0,0 +1,127 @@ +//! Rich task notes in the formatting part (#1801). `Task::rich_notes` is +//! not formatting, but it travels where the formatting does: the `.yppx` +//! `formatting.xml` part and the text a format plugin's `_ex` symbols carry +//! (#1702), so a `.mpp` plugin gets the markup on write and returns it on +//! read without a new ABI symbol. +//! +//! Shape: one `` per task with rich notes, in task +//! order, after the formatting's own children. `markup` is the canonical +//! [`crate::richtext`] markup with the attribute escapes plus ` ` for a +//! newline and ` ` for a tab (an XML reader turns a raw one into a space). +//! The other C0 controls cannot be written in XML 1.0 at all; they are +//! written as they are and read back by this reader only. +//! +//! On read a `` is applied only to an existing, non-blank task whose +//! plain notes are the markup's plain text, so old formatting never lands on +//! other text; any other is dropped. [`Formatting::from_xml`] ignores +//! ``, as an older build does, which then saves the part without them. +//! +//! [`Formatting::from_xml`]: super::Formatting::from_xml + +use super::{PART, decode}; +use crate::json::Json; +use crate::model::Project; +use opccore::xml::{Event, XmlParser}; + +/// The `formatting.xml` text for `proj`: its formatting and the tasks' rich +/// notes, or `None` when there is neither (no part is written, and a plugin +/// gets no formatting text). Without rich notes it is exactly +/// [`super::Formatting::to_xml`]. +pub fn part_xml(proj: &Project) -> Option { + let noted = || proj.tasks.iter().filter(|t| t.rich_notes.is_some()); + if proj.formatting.is_empty() && noted().next().is_none() { + return None; + } + let mut xml = proj.formatting.to_xml(); + let mut notes = String::new(); + for t in noted() { + notes.push_str(" \n"); + } + if !notes.is_empty() { + let end = xml.rfind("").unwrap_or(xml.len()); + xml.insert_str(end, ¬es); + } + Some(xml) +} + +/// ` markup="…"`, escaping what an attribute must keep: the markup's tags, +/// quotes, and its newlines and tabs as character references. +fn push_markup(out: &mut String, value: &str) { + out.push_str(" markup=\""); + for c in value.chars() { + match c { + '&' => out.push_str("&"), + '<' => out.push_str("<"), + '>' => out.push_str(">"), + '"' => out.push_str("""), + '\n' => out.push_str(" "), + '\t' => out.push_str(" "), + '\r' => {} + c => out.push(c), + } + } + out.push('"'); +} + +/// The ``s of a `formatting.xml` text as `(uid, markup)`, in order. +/// A note without a numeric `uid` is skipped. +pub fn read_notes(xml: &str) -> Result, String> { + let mut parser = XmlParser::new(xml); + let mut out = Vec::new(); + let mut depth = 0usize; + loop { + match parser.next() { + Event::Start => { + if depth == 1 && parser.name() == "note" { + let uid = decode(parser.attr("uid")).trim().parse::().ok(); + if let Some(uid) = uid { + out.push((uid, decode(parser.attr("markup")))); + } + } + depth += 1; + } + Event::End => depth = depth.saturating_sub(1), + Event::Text => {} + Event::Eof => break, + } + } + if parser.is_malformed() { + return Err(format!("{PART}: not well-formed XML")); + } + Ok(out) +} + +/// Give `proj`'s tasks the rich notes of a `formatting.xml` text, read after +/// the tasks themselves (MSPDI). A note whose task is missing or blank, whose +/// plain text is not the task's notes, or that formats nothing is dropped. +pub fn apply_notes(proj: &mut Project, xml: &str) -> Result<(), String> { + for (uid, markup) in read_notes(xml)? { + let Some(task) = proj.tasks.iter_mut().find(|t| t.uid == uid && !t.is_null) else { + continue; + }; + let (plain, rich) = crate::richtext::split_notes(&markup); + let notes = task.notes.as_deref().map(crate::text::normalize_newlines); + if rich.is_some() && plain == notes { + task.rich_notes = rich; + } + } + Ok(()) +} + +/// The UI plugin requests' read-only `view.rich_notes`: `{"": markup}` +/// for every task with rich notes. A task not in it has plain notes only +/// (MSPDI ``, to be read as [`crate::richtext::Rich::plain`]). +pub fn rich_notes_json(proj: &Project) -> Json { + Json::Obj( + proj.tasks + .iter() + .filter_map(|t| Some((t.uid.to_string(), Json::str(t.rich_notes.clone()?)))) + .collect(), + ) +} + +#[cfg(test)] +mod tests; diff --git a/projcore/src/formatting/notes/tests.rs b/projcore/src/formatting/notes/tests.rs new file mode 100644 index 000000000..48139a89a --- /dev/null +++ b/projcore/src/formatting/notes/tests.rs @@ -0,0 +1,138 @@ +use super::*; +use crate::formatting::Formatting; +use crate::model::Task; + +fn task(uid: i32, notes: Option<&str>, rich: Option<&str>) -> Task { + Task { + uid, + id: uid, + name: format!("T{uid}"), + notes: notes.map(str::to_string), + rich_notes: rich.map(str::to_string), + ..Task::default() + } +} + +/// Every awkward thing a note can hold: bold, italic, a bullet, a centred +/// line, escapes, a tab, spaces at both ends, an empty middle line. +const RICH: &str = "

    Title

    \n
  • a & <b>\t\"q\"
  • \n

    \n

    end

    "; +const PLAIN: &str = "Title\na & \t\"q\"\n\n end "; + +fn plan() -> Project { + Project { + tasks: vec![ + task(1, Some(PLAIN), Some(RICH)), + task(2, Some("plain only"), None), + task(3, Some("x"), Some("
  • x
  • ")), + ], + ..Project::default() + } +} + +/// The tasks as they come out of MSPDI: plain notes, no markup. +fn stripped(p: &Project) -> Project { + let mut p = p.clone(); + for t in &mut p.tasks { + t.rich_notes = None; + } + p +} + +#[test] +fn notes_round_trip_in_formatting_xml() { + let proj = plan(); + let xml = part_xml(&proj).expect("rich notes alone make a part"); + // Newlines and tabs are character references; no raw one is left in the + // attribute, so a conforming XML reader keeps them. + let line = xml.lines().find(|l| l.contains("uid=\"1\"")).unwrap(); + assert!(line.contains(" ") && line.contains(" "), "{line}"); + assert!(!line.contains('\t')); + assert!(xml.contains(" \n\n")); + assert!( + !xml.contains("uid=\"2\""), + "a plain-only task writes no note" + ); + assert_eq!( + read_notes(&xml).unwrap(), + vec![(1, RICH.to_string()), (3, "
  • x
  • ".to_string())] + ); + let mut back = stripped(&proj); + apply_notes(&mut back, &xml).unwrap(); + assert_eq!(back, proj); +} + +#[test] +fn no_rich_notes_formatting_xml_is_byte_identical() { + let mut proj = stripped(&plan()); + assert_eq!(part_xml(&proj), None, "nothing to write, no part"); + proj.formatting.palette = Some("fx.blue".into()); + proj.formatting.set_switch("flag", Some(true)); + assert_eq!(part_xml(&proj), Some(proj.formatting.to_xml())); +} + +#[test] +fn notes_follow_the_formatting() { + let mut proj = plan(); + proj.formatting.palette = Some("fx.blue".into()); + let xml = part_xml(&proj).unwrap(); + // The formatting reads back as it was; its reader ignores the notes, as + // an older build does. + assert_eq!(Formatting::from_xml(&xml).unwrap(), proj.formatting); + let mut back = stripped(&proj); + apply_notes(&mut back, &xml).unwrap(); + assert_eq!(back.tasks, proj.tasks); +} + +#[test] +fn from_xml_ignores_note_elements() { + let mut f = Formatting::default(); + f.link_style = Some("curved".into()); + let without = f.to_xml(); + let with = without.replace( + "", + " \n", + ); + assert_eq!( + Formatting::from_xml(&with).unwrap(), + Formatting::from_xml(&without).unwrap() + ); +} + +#[test] +fn stale_note_is_dropped_on_apply() { + let xml = part_xml(&plan()).unwrap(); + let mut proj = stripped(&plan()); + // Task 1's text changed since the note was written; task 3 is now a + // blank row; a note for uid 9 has no task. + proj.tasks[0].notes = Some("Title\nsomething else".into()); + proj.tasks[2].is_null = true; + let xml = xml.replace( + "", + " \n", + ); + let before = proj.clone(); + apply_notes(&mut proj, &xml).unwrap(); + assert_eq!(proj, before); + // A note that formats nothing is no rich note, and a stored `\r\n` in the + // task's notes still matches. + let mut proj = Project { + tasks: vec![task(1, Some("a\r\nb"), None), task(2, Some("x"), None)], + ..Project::default() + }; + let xml = "\n \n \n \n\n"; + apply_notes(&mut proj, xml).unwrap(); + assert_eq!( + proj.tasks[0].rich_notes.as_deref(), + Some("
  • a
  • \n

    b

    ") + ); + assert_eq!(proj.tasks[1].rich_notes, None); +} + +#[test] +fn rich_notes_json_lists_the_formatted_tasks_only() { + assert_eq!( + rich_notes_json(&plan()), + Json::obj(vec![("1", Json::str(RICH)), ("3", Json::str("
  • x
  • "))]) + ); + assert_eq!(rich_notes_json(&stripped(&plan())), Json::Obj(Vec::new())); +} diff --git a/projcore/src/yppx.rs b/projcore/src/yppx.rs index 6d869ea1a..5d5c327f1 100644 --- a/projcore/src/yppx.rs +++ b/projcore/src/yppx.rs @@ -64,12 +64,15 @@ pub fn write_yppx(proj: &Project) -> Result, String> { proj.package.unreadable.join(", ") )); } + // The formatting part (#1532) with the tasks' rich notes (#1801), only + // when there is either to keep. + let formatting_xml = formatting::part_xml(proj); let mut entries = vec![ ( CONTENT_TYPES_PART.to_string(), build_content_types( &proj.package, - !proj.formatting.is_empty(), + formatting_xml.is_some(), !proj.table_view.is_empty(), &FLAG_PARTS.map(|f| f.any(&proj.tasks)), ) @@ -103,12 +106,8 @@ pub fn write_yppx(proj: &Project) -> Result, String> { )); } } - // The formatting part (#1532), only when there is formatting to keep. - if !proj.formatting.is_empty() { - entries.push(( - formatting::PART.to_string(), - proj.formatting.to_xml().into_bytes(), - )); + if let Some(xml) = formatting_xml { + entries.push((formatting::PART.to_string(), xml.into_bytes())); } for (name, bytes) in &proj.package.parts { if !name.ends_with('/') && seen.insert(name.to_ascii_lowercase()) { @@ -440,6 +439,7 @@ fn read_yppx_within(bytes: &[u8], budget: u64) -> Result { let text = String::from_utf8(bytes) .map_err(|_| format!("{} is not valid UTF-8", formatting::PART))?; project.formatting = Formatting::from_xml(&text)?; + formatting::apply_notes(&mut project, &text)?; } if let Some(view) = table_view { project.table_view = view; @@ -756,6 +756,45 @@ mod tests { } } + /// Rich task notes (#1801) ride in the formatting part: a note alone + /// writes it (and declares it), a save and reopen keeps the markup, and + /// MSPDI keeps the plain text only. + #[test] + fn rich_notes_survive_yppx_round_trip() { + let mut proj = sample(); + let markup = "

    Bold & more

    \n
  • item
  • "; + proj.tasks[0].notes = Some("Bold & more\nitem".into()); + proj.tasks[0].rich_notes = Some(markup.into()); + proj.tasks[1].notes = Some("plain".into()); + assert!(proj.formatting.is_empty()); + let bytes = write_yppx(&proj).unwrap(); + let zip = ZipArchive::open(&bytes).unwrap(); + let part = String::from_utf8(zip.read(crate::formatting::PART).unwrap()).unwrap(); + assert!(part.contains("Bold & more\nitem
    ")); + assert!(!main.contains("") && !main.contains("<b>")); + let back = read_yppx(&bytes).unwrap(); + assert_eq!(back.tasks[0].rich_notes.as_deref(), Some(markup)); + assert_eq!(back.tasks[1].rich_notes, None); + assert!(back.formatting.is_empty()); + assert!(back.package.parts.is_empty(), "the part is the model's"); + // Saved again, byte for byte. + assert_eq!(write_yppx(&back).unwrap(), bytes); + } + + #[test] + fn plan_without_rich_notes_writes_no_formatting_part() { + let mut proj = sample(); + proj.tasks[0].notes = Some("plain".into()); + let bytes = write_yppx(&proj).unwrap(); + let zip = ZipArchive::open(&bytes).unwrap(); + assert!(zip.read(crate::formatting::PART).is_none()); + assert_eq!(zip.entries().len(), 2); + } + #[test] fn package_is_a_zip() { let bytes = write_yppx(&sample()).unwrap(); diff --git a/projcore/tests/plugins.rs b/projcore/tests/plugins.rs index bce75e73e..28fc21191 100644 --- a/projcore/tests/plugins.rs +++ b/projcore/tests/plugins.rs @@ -163,6 +163,70 @@ fn a_plugin_write_and_read_carry_the_formatting() { let _ = std::fs::remove_dir_all(dir); } +/// Rich task notes (#1801) cross the format plugin seam inside the +/// formatting text: a plan whose only "formatting" is a rich note still hands +/// the plugin a formatting text, and the markup comes back on read. MSPDI +/// alone (a `.xml` save, or the plain read) keeps the plain text only. +#[test] +fn rich_notes_cross_the_plugin_seam_both_ways() { + let dir = fixture_plugin::plugin_dir("projcore-plugin-rich-notes"); + let mut registry = Registry::new(); + for format in load_plugins(std::slice::from_ref(&dir)) { + registry.register(format); + } + let path = Path::new("plan.fakeproj"); + let mut project = corpus_plan(); + assert!(project.formatting.is_empty()); + let i = project + .tasks + .iter() + .position(|t| t.uid != 0 && !t.is_null) + .unwrap(); + let uid = project.tasks[i].uid; + let markup = "

    Bold & plain

    \n
  • item
  • "; + project.tasks[i].notes = Some("Bold & plain\nitem".into()); + project.tasks[i].rich_notes = Some(markup.into()); + + let bytes = registry.write(&project, path, None).unwrap(); + let text = String::from_utf8(bytes.clone()).unwrap(); + assert!(text.contains("