projcore: rich task notes in the plan model (Task, task.set, .yppx, format plugin read/write)

CLAUDE.md

@@ -472,7 +472,9 @@
 - A library may also export `yppx_plugin_read_ex` / `yppx_plugin_write_ex`
   (#1702; independent, optional, ABI still 1) to carry the plan's
   `Formatting`, which MSPDI does not: write_ex gets the `formatting.xml` text
-  beside the MSPDI (NULL/0 when the plan has none) and is called instead of
+  beside the MSPDI (NULL/0 when the plan has neither formatting nor rich
+  notes; its `<note>` children are the rich notes, #1801, which read_ex must
+  return for them to survive) and is called instead of
   `yppx_plugin_write`; read_ex returns it in a second buffer (empty leaves the
   default; non-UTF-8 or unparsable fails the read with the plugin's name).
   `.yppx` keeps it in `formatting.xml` as before. Hosts need nothing: they pass
@@ -853,6 +855,35 @@
   gridlines and clears them on the next press (`project-plugin-ui.uit`, not
   run on macOS).
 
+## Rich task notes (#1801)
+
+- `Task::rich_notes` is the notes' `projcore::richtext` markup beside the
+  plain `notes` every reader uses (MSPDI `<Notes>`, the grid, search, the
+  suite's built-in Notes field): `Some` only when it formats something
+  (bold, italic, a non-left alignment, a bullet), and then its plain text is
+  `notes` (`richtext::split_notes` is the one rule). `Editor::set_rich_notes`
+  sets both as one undo step (a style-only change is a step, the value held
+  none); `set_notes` with new text drops the markup, with the same text
+  keeps it, so a host rewriting unchanged notes loses nothing.
+- It travels where the formatting does: `<note uid markup/>` children of
+  `formatting.xml` after the formatting's own (`formatting::part_xml`, the
+  one writer and "is there a part" test: newlines `&#10;`, tabs `&#9;`), so
+  a `.yppx` save keeps it and a format plugin's `_ex` symbols carry it both
+  ways with no new ABI symbol. A plan without rich notes writes the part
+  byte-identically (or none). On read (`formatting::apply_notes`) a note
+  lands only on a non-blank task whose plain notes it still describes; an
+  older build's `Formatting::from_xml` ignores `<note>`.
+- projctl `task.set {rich_notes}` takes the markup verbatim (never the
+  one-line `notes` form), not with `notes` in one call; every task reply
+  with notes carries `rich_notes` (the stored markup, else the plain notes'
+  own markup, so plain text is never mistaken for markup). UI plugins read
+  it from the read-only `view.rich_notes` (`{uid: markup}`, tasks with
+  formatting only; `formatting::rich_notes_json`) in the suite's
+  `view_json` and yppxy's `run_command`. The fixture's `fixture.richnote`
+  (a command with no control) is the loop, `yppxy/tests/plugins.rs`.
+- Out of scope: resource and assignment notes, rich editing in the
+  built-in Notes field, RTF (the plugin converts).
+
 ## Project leveling as a plan edit (#1594)
 
 - `projcore::schedule::level_with(proj, &LevelScope {movable, order,

PROJECT.md

@@ -436,8 +436,11 @@ refusal is what it was before plugins existed.
   over what the model does not hold. Two more symbols are optional and
   independent (#1702; the ABI stays 1): `yppx_plugin_read_ex` also returns the
   plan's `formatting.xml` in a second buffer, and `yppx_plugin_write_ex` also
-  takes it (NULL and 0 when the plan has none); each is called instead of its
-  plain twin when present, since MSPDI holds no formatting.
+  takes it (NULL and 0 when the plan has neither formatting nor rich task
+  notes); each is called instead of its plain twin when present, since MSPDI
+  holds no formatting. The text also carries the tasks' rich notes as
+  `<note uid markup/>` children (#1801), which read_ex must return (with the
+  MSPDI `<Notes>` equal to their plain text) for them to survive.
 - **`projcore::formats::plugin`**, behind the cargo feature `plugins` (it
   adds `libloading`; hosts enable it, so the default build and the wasm
   builds stay dependency-free), loads the libraries, checks the ABI version

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<bool, String> {
+        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)
     }

projcore/src/editor/cells/tests.rs

@@ -246,6 +246,132 @@ fn set_notes_survive_mspdi_save() {
     assert!(!xml.contains("<Notes>"));
 }
 
+/// 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 = "<p><b>Line 1</b></p>\n<li>Line 2</li>";
+
+#[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, "<p><b>Line </b><b>1</b></p>\n<li>Line 2</li>")
+            .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, "<p>x</p>").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, "<p>a &amp; b</p>\n<p></p>").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 a<b>c\r\nd").unwrap());
+    let task = ed.project().task(10).unwrap();
+    assert_eq!(task.notes.as_deref(), Some("if a<b>c\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, "<li></li>").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();

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)
     }

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";

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 `<note uid="N" markup="…"/>` 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 `&#10;` for a
+//! newline and `&#9;` 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 `<note>` 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
+//! `<note>`, 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<String> {
+    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("  <note");
+        super::push_attr(&mut notes, "uid", &t.uid.to_string());
+        push_markup(&mut notes, t.rich_notes.as_deref().unwrap_or(""));
+        notes.push_str("/>\n");
+    }
+    if !notes.is_empty() {
+        let end = xml.rfind("</formatting>").unwrap_or(xml.len());
+        xml.insert_str(end, &notes);
+    }
+    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("&amp;"),
+            '<' => out.push_str("&lt;"),
+            '>' => out.push_str("&gt;"),
+            '"' => out.push_str("&quot;"),
+            '\n' => out.push_str("&#10;"),
+            '\t' => out.push_str("&#9;"),
+            '\r' => {}
+            c => out.push(c),
+        }
+    }
+    out.push('"');
+}
+
+/// The `<note>`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<Vec<(i32, String)>, 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::<i32>().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`: `{"<uid>": markup}`
+/// for every task with rich notes. A task not in it has plain notes only
+/// (MSPDI `<Notes>`, 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;