| author | |
| committer | |
| log | 8dcd2f78650d5b4ebd9d18efaf37e73583c54de9 |
| tree | 69b34d2e1d0313da46a3451e7e8bde4f7930c657 |
| parent | 68d9fb0eeb2d423cdf9254cec4919e7e84627ca7 |
| signature | Signed by SSH key SHA256:52mNGHRsVFBDED9IAX5pe+LRWUefqTbxEReunq21QvU (~clover) |
Attribute changed page spaces when section indentation edits store metadata,
leaving unchanged pages and no-op edits untouched. Advance the page freshness
watermark when explicitly marking it unread so an earlier own-change callback
cannot clear the mark. Keep the existing toggle with its read state and exercise
both storage and callback ordering with regressions.
Fixes #107
Assisted-by: gpt-6.1-sol4 files changed, 111 insertions(+), 25 deletions(-)
crates/notebook/tests/session.rs+58| ... | @@ -1636,3 +1636,61 @@ fn text_edits_name_the_latest_author_even_within_the_same_second() { | ... | @@ -1636,3 +1636,61 @@ fn text_edits_name_the_latest_author_even_within_the_same_second() { |
| 1636 | .unwrap(); | 1636 | .unwrap(); |
| 1637 | } | 1637 | } |
| 1638 | } | 1638 | } |
| 1639 | |||
| 1640 | #[test] | ||
| 1641 | fn page_level_edits_attribute_only_changed_pages_and_skip_noops() { | ||
| 1642 | let image = onestore::create_section("Levels.one", "Parent", "Alice").unwrap(); | ||
| 1643 | let arena = onestore::Arena::default(); | ||
| 1644 | let mut section = onestore::Section::open(&arena, image).unwrap(); | ||
| 1645 | let creation = onestore::PageCreation::new(None, Some("Child"), "Alice").unwrap(); | ||
| 1646 | let child = creation.space(); | ||
| 1647 | section | ||
| 1648 | .apply( | ||
| 1649 | "Alice", | ||
| 1650 | &Edit { | ||
| 1651 | at: model_ops::now(), | ||
| 1652 | ops: vec![Op::Section(onestore::op::SectionOp::Create(creation))], | ||
| 1653 | }, | ||
| 1654 | ) | ||
| 1655 | .unwrap(); | ||
| 1656 | section.seal().unwrap().unwrap(); | ||
| 1657 | let before = section.image(); | ||
| 1658 | let pages = notebook::session::stored_pages(&before).unwrap(); | ||
| 1659 | let at = pages.iter().filter_map(|page| page.modified).max().unwrap() + 12345; | ||
| 1660 | let store = onestore::Store::parse(&before).unwrap(); | ||
| 1661 | let index = onestore::RevisionIndex::parse(&store).unwrap(); | ||
| 1662 | let revisions = index.spaces[&child].revisions.len(); | ||
| 1663 | let edit = Edit { | ||
| 1664 | at, | ||
| 1665 | ops: vec![Op::Section(onestore::op::SectionOp::Pages(vec![ | ||
| 1666 | onestore::PageEdit::set_level(child, 2).unwrap(), | ||
| 1667 | ]))], | ||
| 1668 | }; | ||
| 1669 | section.apply("Bob", &edit).unwrap(); | ||
| 1670 | section.seal().unwrap().unwrap(); | ||
| 1671 | let image = section.image(); | ||
| 1672 | let after = notebook::session::stored_pages(&image).unwrap(); | ||
| 1673 | assert_eq!(after.len(), 2); | ||
| 1674 | for (before, after) in pages.iter().zip(&after) { | ||
| 1675 | if after.space == child { | ||
| 1676 | assert_eq!(after.author.as_deref(), Some("Bob")); | ||
| 1677 | assert_eq!(after.modified, Some(at)); | ||
| 1678 | } else { | ||
| 1679 | assert_eq!(after.author, before.author); | ||
| 1680 | assert_eq!(after.modified, before.modified); | ||
| 1681 | } | ||
| 1682 | } | ||
| 1683 | assert_eq!(section.pages().unwrap()[1].2, 2); | ||
| 1684 | let store = onestore::Store::parse(&image).unwrap(); | ||
| 1685 | let index = onestore::RevisionIndex::parse(&store).unwrap(); | ||
| 1686 | assert_eq!(index.spaces[&child].revisions.len(), revisions + 1); | ||
| 1687 | section | ||
| 1688 | .apply("Nobody", &Edit { at: at + 1, ..edit }) | ||
| 1689 | .unwrap(); | ||
| 1690 | assert!(section.seal().unwrap().is_none()); | ||
| 1691 | assert_eq!(section.image(), image); | ||
| 1692 | if let Some(folder) = std::env::var_os("SNOWBOUND_AUTHOR_EXPORT") { | ||
| 1693 | std::fs::create_dir_all(&folder).unwrap(); | ||
| 1694 | std::fs::write(std::path::Path::new(&folder).join("Levels.one"), image).unwrap(); | ||
| 1695 | } | ||
| 1696 | } |
crates/onestore/src/op/apply.rs+3-1| ... | @@ -157,7 +157,9 @@ impl<'a> Section<'a> { | ... | @@ -157,7 +157,9 @@ impl<'a> Section<'a> { |
| 157 | } | 157 | } |
| 158 | SectionOp::Pages(edits) => { | 158 | SectionOp::Pages(edits) => { |
| 159 | for (space, changes) in crate::pages::section_changes(self, edits, &[])? { | 159 | for (space, changes) in crate::pages::section_changes(self, edits, &[])? { |
| 160 | self.apply_changes(space, &[], changes)?; | 160 | if self.apply_changes(space, &[], changes)? && space != self.root() { |
| 161 | self.attribute(space, author, Vec::new())?; | ||
| 162 | } | ||
| 161 | } | 163 | } |
| 162 | } | 164 | } |
| 163 | SectionOp::Delete(pages) => { | 165 | SectionOp::Delete(pages) => { |
crates/snowbound/src/commands.rs+1-1| ... | @@ -1556,7 +1556,7 @@ impl State { | ... | @@ -1556,7 +1556,7 @@ impl State { |
| 1556 | return Ok(()); | 1556 | return Ok(()); |
| 1557 | } | 1557 | } |
| 1558 | Id::MarkRead => { | 1558 | Id::MarkRead => { |
| 1559 | self.toggle_read(); | 1559 | self.reads.toggle_read(); |
| 1560 | return Ok(()); | 1560 | return Ok(()); |
| 1561 | } | 1561 | } |
| 1562 | Id::NextUnread => { | 1562 | Id::NextUnread => { |
crates/snowbound/src/unread.rs+49-23| ... | @@ -149,6 +149,28 @@ impl Reads { | ... | @@ -149,6 +149,28 @@ impl Reads { |
| 149 | self.save(); | 149 | self.save(); |
| 150 | } | 150 | } |
| 151 | 151 | ||
| 152 | /// Mark as Read, or as Unread, on the page shown, as OneNote 2010's Ctrl+Q does. | ||
| 153 | pub(crate) fn toggle_read(&mut self) { | ||
| 154 | let Some((notebook, section, space, _)) = self.viewing.clone() else { | ||
| 155 | return; | ||
| 156 | }; | ||
| 157 | if self | ||
| 158 | .pages(&notebook, section) | ||
| 159 | .is_some_and(|pages| pages.contains(&space)) | ||
| 160 | { | ||
| 161 | self.mark_read(&notebook, section, Some(space)); | ||
| 162 | } else { | ||
| 163 | let kept = self.section_mut(&notebook, section); | ||
| 164 | kept.unread.insert(space); | ||
| 165 | let seen = kept.pages.entry(space).or_default(); | ||
| 166 | *seen = (*seen).max(now()); | ||
| 167 | if let Some((.., kept)) = &mut self.viewing { | ||
| 168 | *kept = true; | ||
| 169 | } | ||
| 170 | self.save(); | ||
| 171 | } | ||
| 172 | } | ||
| 173 | |||
| 152 | /// Takes in a section as its file stands, its pages with their FILETIME and | 174 | /// Takes in a section as its file stands, its pages with their FILETIME and |
| 153 | /// whether the reader changed them last: those another author changed since it was last | 175 | /// whether the reader changed them last: those another author changed since it was last |
| 154 | /// known are unread. | 176 | /// known are unread. |
| ... | @@ -346,29 +368,6 @@ impl State { | ... | @@ -346,29 +368,6 @@ impl State { |
| 346 | }); | 368 | }); |
| 347 | } | 369 | } |
| 348 | 370 | ||
| 349 | /// Mark as Read, or as Unread, on the page shown, as OneNote 2010's Ctrl+Q does. | ||
| 350 | pub(crate) fn toggle_read(&mut self) { | ||
| 351 | let Some((notebook, section, space, _)) = self.reads.viewing.clone() else { | ||
| 352 | return; | ||
| 353 | }; | ||
| 354 | if self | ||
| 355 | .reads | ||
| 356 | .pages(&notebook, section) | ||
| 357 | .is_some_and(|pages| pages.contains(&space)) | ||
| 358 | { | ||
| 359 | self.reads.mark_read(&notebook, section, Some(space)); | ||
| 360 | } else { | ||
| 361 | self.reads | ||
| 362 | .section_mut(&notebook, section) | ||
| 363 | .unread | ||
| 364 | .insert(space); | ||
| 365 | if let Some((.., kept)) = &mut self.reads.viewing { | ||
| 366 | *kept = true; | ||
| 367 | } | ||
| 368 | self.reads.save(); | ||
| 369 | } | ||
| 370 | } | ||
| 371 | |||
| 372 | /// Whether the page shown is unread. | 371 | /// Whether the page shown is unread. |
| 373 | pub(crate) fn page_unread(&self) -> bool { | 372 | pub(crate) fn page_unread(&self) -> bool { |
| 374 | self.reads | 373 | self.reads |
| ... | @@ -607,6 +606,33 @@ mod tests { | ... | @@ -607,6 +606,33 @@ mod tests { |
| 607 | notebook::fs::remove_dir_all(&folder).unwrap(); | 606 | notebook::fs::remove_dir_all(&folder).unwrap(); |
| 608 | } | 607 | } |
| 609 | 608 | ||
| 609 | #[test] | ||
| 610 | fn explicitly_unread_page_rejects_earlier_own_callbacks() { | ||
| 611 | let folder = | ||
| 612 | std::env::temp_dir().join(format!("snowbound-unread-explicit-{}", std::process::id())); | ||
| 613 | notebook::fs::create_dir_all(&folder).unwrap(); | ||
| 614 | let (notebook, section) = ("/local-notebook", [3; 16]); | ||
| 615 | let mut reads = Reads::load(&folder); | ||
| 616 | let at = now() - 100_000_000; | ||
| 617 | reads.section_mut(notebook, section).read = at; | ||
| 618 | let first = page(1, None).0; | ||
| 619 | reads.arrived(notebook, section, [(first, Some(at + 1), true)]); | ||
| 620 | reads.viewing = Some((notebook.into(), section, first, false)); | ||
| 621 | reads.toggle_read(); | ||
| 622 | reads.arrived(notebook, section, [(first, Some(at + 2), true)]); | ||
| 623 | assert!(reads.pages(notebook, section).unwrap().contains(&first)); | ||
| 624 | assert!(reads.viewing.as_ref().unwrap().3); | ||
| 625 | let mut reads = Reads::load(&folder); | ||
| 626 | reads.arrived(notebook, section, [(first, Some(at + 2), true)]); | ||
| 627 | assert!(reads.pages(notebook, section).unwrap().contains(&first)); | ||
| 628 | let marked = reads.section_mut(notebook, section).pages[&first]; | ||
| 629 | reads.arrived(notebook, section, [(first, Some(marked + 1), true)]); | ||
| 630 | assert!(reads.pages(notebook, section).unwrap().is_empty()); | ||
| 631 | reads.arrived(notebook, section, [page(1, Some(marked + 2))]); | ||
| 632 | assert!(reads.pages(notebook, section).unwrap().contains(&first)); | ||
| 633 | notebook::fs::remove_dir_all(&folder).unwrap(); | ||
| 634 | } | ||
| 635 | |||
| 610 | #[test] | 636 | #[test] |
| 611 | fn legacy_time32_read_watermarks_load_without_losing_unread_pages() { | 637 | fn legacy_time32_read_watermarks_load_without_losing_unread_pages() { |
| 612 | let folder = | 638 | let folder = |