From c414df0810a16db7b85ef4c2abddc5dbd289154b Mon Sep 17 00:00:00 2001 From: clover caruso Date: Thu, 15 Oct 2026 02:54:23 -0700 Subject: [PATCH] feat(mobile): keep refused edits as conflict pages When the section refuses an edit, iOS keeps the page as the view shows it instead of putting its text on the clipboard: as a conflict page under the stored page with its differing paragraphs highlighted, as OneNote 2010 keeps a version it could not merge, or as a copy where the page is gone. The alert mentions the kept page only once the section has taken it. Refs #136 Assisted-by: claude-opus-5.5 --- apps/ios/Snowbound/CanvasView.swift | 3 +- apps/ios/Snowbound/PageViewController.swift | 6 +- crates/mobile/include/snowbound.h | 2 +- crates/mobile/src/lib.rs | 71 +++++---- crates/mobile/src/tests.rs | 150 ++++++++++++++++++++ crates/notebook/src/merge.rs | 105 +++++++++----- crates/notebook/src/session.rs | 1 + 7 files changed, 273 insertions(+), 65 deletions(-) diff --git a/apps/ios/Snowbound/CanvasView.swift b/apps/ios/Snowbound/CanvasView.swift index 3a8be64af32d951adb7e3d829d5b078cd9dae2ee..7482270e0f026b55e118d5cb6fda02a214b3b2e3 100644 --- a/apps/ios/Snowbound/CanvasView.swift +++ b/apps/ios/Snowbound/CanvasView.swift @@ -1050,7 +1050,8 @@ final class CanvasView: UIScrollView, UIScrollViewDelegate, UITextInput, UITextI /// The title as typed; nil on a page without an editable title. var pageTitle: String? { handle.flatMap { take(sb_view_title($0)) } } - var pageText: String { handle.flatMap { take(sb_view_page_text($0)) } ?? "" } + /// Keeps the page as shown as a page of its own after an edit was refused; false when nothing was kept. + func keep() -> Bool { handle.map(sb_view_keep) ?? false } /// The page on `paper`, in points, as OneNote 2010 prints it, each sheet's footer naming /// `section`. diff --git a/apps/ios/Snowbound/PageViewController.swift b/apps/ios/Snowbound/PageViewController.swift index ad1fcb272355d0190728b036963312e00c84a38f..2feeb966c28928f5f8126e1107cd9d6f6f5b2569 100644 --- a/apps/ios/Snowbound/PageViewController.swift +++ b/apps/ios/Snowbound/PageViewController.swift @@ -409,10 +409,10 @@ final class PageViewController: UIViewController, PHPickerViewControllerDelegate @objc private func sectionChanged(_ notification: Notification) { let flags = notification.userInfo?["flags"] as? UInt32 ?? 0 if flags & Section.rejected != 0 { - // Nothing typed is lost unannounced: the page's text goes on the clipboard. - UIPasteboard.general.string = canvas.pageText + let kept = canvas.keep() canvas.reload(discard: true) - alert("Change Not Saved", "The page shows what was last saved. Your text is on the clipboard to paste back.") + alert("Change Not Saved", "The page shows what was last saved." + + (kept ? " Your version is kept as a separate page." : "")) } else if flags & Section.remote != 0 { canvas.reload(discard: false) } else { diff --git a/crates/mobile/include/snowbound.h b/crates/mobile/include/snowbound.h index b6b4148054aa8d170116cee288fe687b48ba5b44..54c37c870335cd38871a5b1038dc0750e81f0624 100644 --- a/crates/mobile/include/snowbound.h +++ b/crates/mobile/include/snowbound.h @@ -105,6 +105,7 @@ bool sb_view_set_reading(View *view, float width, float anchor[2]); bool sb_view_reload_ready(const View *view); // A non-null page-point anchor follows its source caret across the refresh. bool sb_view_reload(View *view, bool discard, float anchor[2]); +bool sb_view_keep(const View *view); void sb_view_focus(View *view, bool focused); void sb_view_content(View *view, float bounds[4]); bool sb_view_block(const View *view, float x, float y, float rect[4]); @@ -137,7 +138,6 @@ bool sb_view_ink_selection(const View *view, float rect[4]); char *sb_view_paper(const View *view); bool sb_view_set_paper(View *view, int16_t red, uint8_t green, uint8_t blue, int8_t ruled); bool sb_view_set_art(View *view, const char *name); -char *sb_view_page_text(const View *view); uint8_t *sb_view_pdf(const View *view, float width, float height, const char *section, size_t *length); void sb_bytes_free(uint8_t *bytes, size_t length); int8_t sb_view_date_request(View *view, int64_t *seconds); diff --git a/crates/mobile/src/lib.rs b/crates/mobile/src/lib.rs index 2380aa3f3484aa673913a23af004b52ed57e5022..5e1044a8d7fc9f85d554e101c07869558265a851 100644 --- a/crates/mobile/src/lib.rs +++ b/crates/mobile/src/lib.rs @@ -31,10 +31,11 @@ use draw::edit::{Key, NamedKey, SelectionUnit}; use onestore::{ ExGuid, op::{Edit, Op, PageOp}, - page::{Page, ink::ShapeKind}, + page::{Page, PageObject, PageParagraph, ink::ShapeKind}, }; use parley::Affinity; use std::{ + collections::BTreeSet, error::Error, ffi::{CStr, CString, c_char, c_void}, mem::ManuallyDrop, @@ -460,26 +461,6 @@ impl Canvas { Ok(true) } - /// The page's text, outline by outline, as the desktop puts a refused edit's on the - /// clipboard. - fn page_text(&self) -> String { - let editor = &self.page.editor; - editor - .visible_outlines() - .chain(editor.caret_outline()) - .map(|outline| { - outline - .document() - .paragraphs() - .map(canvas::search::shown) - .collect::>() - .join("\n") - }) - .filter(|text| !text.trim().is_empty()) - .collect::>() - .join("\n\n") - } - /// Gives the page `color`, a COLORREF, and rule lines `ruled` of `RULE_LINES`, or none. fn set_paper(&mut self, color: Option, ruled: Option) -> Result { let rules = ruled @@ -1238,6 +1219,48 @@ pub extern "C" fn sb_view_reload_ready(view: &View) -> bool { .is_ok_and(|edits| edits.pending == 0 && !edits.failed) } +/// After the section refused an edit, keeps the page as the view shows it, as OneNote keeps a +/// version it could not merge: a conflict page under the stored page, its differing paragraphs +/// highlighted, or a copy of it where the page is gone. True once the section took it. +#[unsafe(no_mangle)] +pub extern "C" fn sb_view_keep(view: &View) -> bool { + let kept = (|| -> Result { + let space = view.canvas.space; + let page = view.canvas.page.editor.page()?; + let section = &view.section.section; + let stored = section.page(space).ok(); + if stored.as_ref() == Some(&page) { + return Ok(false); + } + let paragraphs = |page: &Page| -> Vec { + page.objects + .iter() + .flat_map(|object| match object { + PageObject::Outline(outline) => outline.paragraphs.clone(), + _ => Vec::new(), + }) + .collect() + }; + let before = stored.as_ref().map(paragraphs).unwrap_or_default(); + let objects: BTreeSet = paragraphs(&page) + .into_iter() + .filter(|paragraph| !before.contains(paragraph)) + .map(|paragraph| paragraph.id) + .collect(); + let author = view.section.author(); + let op = notebook::session::conflict(space, &page, &objects, &author, stored.is_some())?; + section.replica().apply( + &author, + Edit { + at: library::filetime(), + ops: vec![op], + }, + )?; + Ok(true) + })(); + report(kept).unwrap_or(false) +} + /// Shows the page as stored after a change made elsewhere, keeping the caret; with /// `discard`, after the section refused an edit, drops what the page took since. /// A supplied page-point anchor follows its source caret across the refresh. @@ -1664,12 +1687,6 @@ pub unsafe extern "C" fn sb_tag_icon( true } -/// The page's text, for the clipboard when an edit is refused; freed with `sb_string_free`. -#[unsafe(no_mangle)] -pub extern "C" fn sb_view_page_text(view: &View) -> *mut c_char { - owned(view.canvas.page_text()) -} - /// The page on paper `width` × `height` points as OneNote 2010 prints it, each sheet's /// footer naming `section`: `length` bytes of PDF, freed with `sb_bytes_free`, or null. /// diff --git a/crates/mobile/src/tests.rs b/crates/mobile/src/tests.rs index 01a62b4d8c196c90a5922ec421af8a7e78498017..a2a6e4dc6530747c85da03fcd4099d225d16f3d0 100644 --- a/crates/mobile/src/tests.rs +++ b/crates/mobile/src/tests.rs @@ -573,6 +573,156 @@ fn conflict_pages_list_under_their_page_and_take_no_edits() { assert!(highlighted); } +#[test] +fn a_refused_edit_is_kept_as_a_conflict_page_or_a_copy() { + use onestore::page::PageObject; + + let directory = tempfile::tempdir().unwrap(); + let file = directory.path().join("Keep.one"); + let cache = directory.path().join("cache"); + std::fs::write( + &file, + onestore::create_section("Keep.one", "Original body", "Fixture").unwrap(), + ) + .unwrap(); + let library = Arc::new(Library::open(&file, &cache, true).unwrap()); + let section = Section::open( + Arc::clone(&library), + file.to_str().unwrap(), + "Fixture".into(), + || {}, + ) + .unwrap(); + let space = section.shared.section.pages().unwrap()[0].0; + let (page, read_only) = section.shared.page(space).unwrap(); + let outline = page + .objects + .iter() + .find_map(|object| match object { + PageObject::Outline(outline) => Some(outline.id), + _ => None, + }) + .unwrap(); + let mut view = View { + canvas: ManuallyDrop::new(Canvas::new(space, page, [640, 960], 2.0).unwrap()), + presentation: None, + frame: Arc::default(), + section: Arc::clone(§ion.shared), + read_only, + reload_pending: false, + local_edits: Arc::new(Mutex::new(LocalEdits::default())), + }; + focus(&mut view.canvas, "Original body"); + assert!(sb_select(&mut view, 0, 0)); + // Another writer deletes the outline the view still shows. + section + .shared + .section + .apply( + "Peer", + page_edit(space, vec![PageOp::Delete { object: outline }]), + ) + .unwrap(); + section.shared.section.written().unwrap(); + if sb_view_format(&view) & 1 == 0 { + assert!(sb_view_apply(&mut view, 0)); + } + assert!(unsafe { sb_insert(&mut view, c"Stale rich 😀 ".as_ptr()) }); + let deadline = Instant::now() + Duration::from_secs(10); + while section.poll() & 4 == 0 { + assert!(Instant::now() < deadline); + std::thread::sleep(Duration::from_millis(10)); + } + assert!(sb_view_keep(&view)); + assert!(sb_view_reload(&mut view, true, None)); + assert!(!sb_view_keep(&view)); + section.shared.section.written().unwrap(); + let kept = |conflicts: Vec<(ExGuid, Vec)>| { + let [(of, versions)] = conflicts.as_slice() else { + panic!("{conflicts:?}"); + }; + assert_eq!(*of, space); + let [version] = versions.as_slice() else { + panic!("{versions:?}"); + }; + assert_eq!(version.user, "Fixture"); + version.space + }; + let version = kept(section.shared.section.conflicts().unwrap()); + let (page, read_only) = section.shared.page(version).unwrap(); + assert!(read_only); + let stale = page.objects.iter().any(|object| match object { + PageObject::Outline(outline) => outline.paragraphs.iter().any(|paragraph| { + paragraph.format.highlight == Some(canvas::conflict::CONFLICTING) + && match ¶graph.content { + onestore::page::ParagraphContent::Text(text) => { + text.text.text().starts_with("Stale rich 😀 Original body") + && text.text.spans()[0].format.bold == Some(true) + } + _ => false, + } + }), + _ => false, + }); + assert!(stale, "{page:?}"); + let shared = Arc::clone(§ion.shared); + drop((view, section, library)); + Arc::into_inner(shared) + .expect("No other writer") + .section + .close() + .unwrap(); + let library = Arc::new(Library::open(&file, &cache, true).unwrap()); + let section = open(&library, "Keep.one"); + assert_eq!(kept(section.shared.section.conflicts().unwrap()), version); + + // A change outside the text, to a page deleted meanwhile, is kept as a copy of the page. + let (page, read_only) = section.shared.page(space).unwrap(); + let title = page.title.clone(); + let mut view = View { + canvas: ManuallyDrop::new(Canvas::new(space, page, [640, 960], 2.0).unwrap()), + presentation: None, + frame: Arc::default(), + section: Arc::clone(§ion.shared), + read_only, + reload_pending: false, + local_edits: Arc::new(Mutex::new(LocalEdits::default())), + }; + section + .shared + .section + .apply( + "Peer", + Edit { + at: library::filetime(), + ops: vec![Op::Section(onestore::op::SectionOp::Delete(vec![space]))], + }, + ) + .unwrap(); + section.shared.section.written().unwrap(); + assert!(sb_view_set_paper(&mut view, 0x77, 0x88, 0x99, -1)); + let deadline = Instant::now() + Duration::from_secs(10); + while section.poll() & 4 == 0 { + assert!(Instant::now() < deadline); + std::thread::sleep(Duration::from_millis(10)); + } + assert!(sb_view_keep(&view)); + let copies: Vec<_> = section + .shared + .section + .pages() + .unwrap() + .into_iter() + .filter(|(listed, _, _)| *listed != space) + .map(|(listed, _, _)| section.shared.section.page(listed).unwrap()) + .collect(); + let [copy] = copies.as_slice() else { + panic!("{copies:?}"); + }; + assert_eq!(copy.color, Some(0x0099_8877)); + assert_eq!(copy.title, title); +} + #[test] fn search_finds_pages_by_title_and_text_across_sections() { let directory = tempfile::tempdir().unwrap(); diff --git a/crates/notebook/src/merge.rs b/crates/notebook/src/merge.rs index cd5d59645950181c300e3d1ca1b238cca132a95b..7f79119c7e91599db1c92c0f175549a21585517f 100644 --- a/crates/notebook/src/merge.rs +++ b/crates/notebook/src/merge.rs @@ -203,36 +203,9 @@ pub(crate) fn conflict_page( at: u64, guid: Option<[u8; 16]>, ) -> Result { - let placed = |creation: PageCreation| match guid { - Some(guid) => creation.in_space(guid), - None => Ok(creation), - }; - let mut page = page.clone(); - page.objects - .retain(|object| !matches!(object, PageObject::Unsupported(_))); let listed = order(new)?; let ops = if listed.iter().any(|(listed, _)| *listed == space) { - let index = Index::of(&page); - let mut marked: Vec = objects - .iter() - .copied() - .filter(|id| index.nodes.contains_key(id)) - .collect(); - let copy = page.copy_with(&mut marked)?; - let titled = page - .objects - .iter() - .any(|object| matches!(object, PageObject::Title(_))); - vec![Op::Section(SectionOp::Conflict { - of: space, - creation: placed(PageCreation::new( - None, - titled.then_some(page.title.as_str()), - author, - )?)?, - page: copy, - objects: marked, - })] + vec![conflict_op(space, page, objects, author, guid)?] } else { // A copy under fresh identities, as OneNote 2010 puts a removed page back. let queued = order(local)?; @@ -243,11 +216,11 @@ pub(crate) fn conflict_page( .map(|(next, _)| *next) .find(|next| !moved.contains(next) && listed.iter().any(|(listed, _)| listed == next)); let before = series_start(new, before)?; - let creation = placed(PageCreation::new(before, Some(&page.title), author)?)?; - let mut ops = vec![Op::Section(SectionOp::Import { - creation: creation.clone(), - page: page.copy()?, - })]; + let mut creation = PageCreation::new(before, Some(&page.title), author)?; + if let Some(guid) = guid { + creation = creation.in_space(guid)?; + } + let mut ops = vec![import(page, creation.clone())?]; if let Some(level) = at.map(|at| queued[at].1).filter(|level| *level > 1) { ops.push(Op::Section(SectionOp::Pages(vec![PageEdit::set_level( creation.space(), @@ -261,6 +234,72 @@ pub(crate) fn conflict_page( Ok(edit) } +/// The op keeping `page`, a version of the page in `space` the section could not take: a +/// conflict page under it with `objects` highlighted, or, where the section no longer lists +/// the page, a copy of it at the end under fresh identities. Content outside the page model +/// stays behind. +pub fn conflict( + space: ExGuid, + page: &Page, + objects: &BTreeSet, + author: &str, + listed: bool, +) -> Result { + if listed { + conflict_op(space, page, objects, author, None) + } else { + import(page, PageCreation::new(None, Some(&page.title), author)?) + } +} + +/// `page` as the page model holds it, without content outside it. +fn modelled(page: &Page) -> Page { + let mut page = page.clone(); + page.objects + .retain(|object| !matches!(object, PageObject::Unsupported(_))); + page +} + +fn import(page: &Page, creation: PageCreation) -> Result { + Ok(Op::Section(SectionOp::Import { + creation, + page: modelled(page).copy()?, + })) +} + +/// The conflict page keeping `page` under the page in `space`, marking `objects`, in the space +/// `guid` names or a fresh one. +fn conflict_op( + space: ExGuid, + page: &Page, + objects: &BTreeSet, + author: &str, + guid: Option<[u8; 16]>, +) -> Result { + let page = modelled(page); + let index = Index::of(&page); + let mut marked: Vec = objects + .iter() + .copied() + .filter(|id| index.nodes.contains_key(id)) + .collect(); + let copy = page.copy_with(&mut marked)?; + let titled = page + .objects + .iter() + .any(|object| matches!(object, PageObject::Title(_))); + let mut creation = PageCreation::new(None, titled.then_some(page.title.as_str()), author)?; + if let Some(guid) = guid { + creation = creation.in_space(guid)?; + } + Ok(Op::Section(SectionOp::Conflict { + of: space, + creation, + page: copy, + objects: marked, + })) +} + /// Pages the base and the remote both list that the remote moved: into another series or /// level, or out of base order among the rest. OneNote 2010 and this writer give a moved /// page a series of its own; older builds of this writer moved a page within its series. diff --git a/crates/notebook/src/session.rs b/crates/notebook/src/session.rs index 9ad4304e16ba3e6ee362794e02682fa66d0d5714..3e14e308092b9b8750e95d84a51221370fcff793 100644 --- a/crates/notebook/src/session.rs +++ b/crates/notebook/src/session.rs @@ -2,6 +2,7 @@ //! publishes their edits to the section file in the background. pub use crate::background::{Background, Known, Listener}; +pub use crate::merge::conflict; use crate::{ EditStatus, Error, PendingEdit, Remote, Replica, Resolution, Result, SyncWorker, discover, fs, }; -- 2.54.0