authorgravatar for git@paperclover.netclover caruso <git@paperclover.net> 2026-10-02 18:00:13-07:00
committergravatar for git@paperclover.netclover caruso <git@paperclover.net> 2026-10-02 19:55:46-07:00
logb27d10a6a06d758d5d362f437f3080857dc9918c
tree64e334fc63bca5d5611924b1462cb258f4a472fd
parentb58ce1faf590fd325dd35a8489ce67900b202c38
signature Signed by SSH key SHA256:52mNGHRsVFBDED9IAX5pe+LRWUefqTbxEReunq21QvU

fix: Undo takes back theme changes, section colours and notebook properties

Saving in the Themes dialog is a step on the window's undo timeline: Undo puts the themes and the scope's theme back and restyles the page, while what it wrote still holds. A section's colour and Notebook Properties' name and colour go back too. Assisted-by: claude-opus-5.5

2 files changed, 283 insertions(+), 3 deletions(-)

crates/snowbound/src/themes.rs+13-2
......@@ -681,10 +681,21 @@ impl State {
681681 theme: apply.then(|| chosen.id.clone()),
682682 assigned: now,
683683 };
684 dialog.library.save_themes(Themes {
684 let wrote = Themes {
685685 themes,
686686 assignments: vec![assignment],
687 });
687 };
688 let before = dialog.library.themes();
689 dialog.library.save_themes(wrote.clone());
690 // A page's theme is taken back in its section, others anywhere in the notebook.
691 let section = (dialog.targets[at].0 == Scope::Page)
692 .then(|| {
693 let session = self.session.as_ref()?;
694 (session.library).section_identity(&session.tabs[session.tab].path)
695 })
696 .flatten();
697 let undo = crate::undo::themes_undo(&dialog.library, section, &before, wrote);
698 self.undo.record(undo);
688699 }
689700 self.ui.close_popup(id());
690701 self.themes = None;
crates/snowbound/src/undo.rs+270-1
......@@ -10,6 +10,7 @@ use notebook::{
1010 Replica,
1111 discover::Folder,
1212 session::{arrange, blank},
13 sidecar::themes::{Theme, Themes},
1314};
1415use onestore::{
1516 ExGuid, PageCreation, PageEdit,
......@@ -106,6 +107,19 @@ pub enum Change {
106107 folder: Option<[u8; 16]>,
107108 order: Vec<[u8; 16]>,
108109 },
110 /// Colours section `entry` `to` while it is coloured `from`.
111 Recolor {
112 entry: [u8; 16],
113 from: Option<u32>,
114 to: Option<u32>,
115 },
116 /// Names the notebook `to` and colours it, where `to` has a colour, while it is `from`.
117 Properties {
118 from: (String, Option<u32>),
119 to: (String, Option<u32>),
120 },
121 /// Writes the notebook's themes and assignments `to` while those `from` wrote still hold.
122 Themes { from: Themes, to: Themes },
109123}
110124
111125/// A page taken out of its section, and where it was: before `before` at `level`.
......@@ -607,7 +621,11 @@ impl Site {
607621 gone,
608622 }
609623 }
610 Change::Rename { .. } | Change::Place { .. } => {
624 Change::Rename { .. }
625 | Change::Place { .. }
626 | Change::Recolor { .. }
627 | Change::Properties { .. }
628 | Change::Themes { .. } => {
611629 unreachable!("a section's own change is the notebook's")
612630 }
613631 }))
......@@ -692,6 +710,15 @@ pub fn structure_undo(library: &Library, change: &crate::manage::Structure) -> O
692710 order: entries(folder).into_iter().map(|(id, _)| id).collect(),
693711 }
694712 }
713 Structure::Color { path, color } => Change::Recolor {
714 entry: identity_at(catalog, path)?,
715 from: *color,
716 to: library.section_color(path),
717 },
718 Structure::Properties { name, color } => Change::Properties {
719 from: (name.clone(), *color),
720 to: (library.name.clone(), library.color()),
721 },
695722 _ => return None,
696723 };
697724 Some(Action {
......@@ -701,6 +728,62 @@ pub fn structure_undo(library: &Library, change: &crate::manage::Structure) -> O
701728 })
702729}
703730
731/// Whether `current` holds the themes and assignments `wrote`, whatever their timestamps.
732fn holds(current: &Themes, wrote: &Themes) -> bool {
733 let same =
734 |a: &Theme, b: &Theme| (&a.name, &a.styles, a.deleted) == (&b.name, &b.styles, b.deleted);
735 wrote.themes.iter().all(|wrote| {
736 (current.themes.iter())
737 .find(|kept| kept.id == wrote.id)
738 .is_some_and(|kept| same(kept, wrote))
739 }) && wrote.assignments.iter().all(|wrote| {
740 let kept = (current.assignments.iter()).find(|kept| kept.scope == wrote.scope);
741 kept.and_then(|kept| kept.theme.as_ref()) == wrote.theme.as_ref()
742 })
743}
744
745/// The action taking back `wrote`, just written to `library`'s themes, which held `before`
746/// until then: the themes as they were, a new one deleted, and each scope's theme.
747pub fn themes_undo(
748 library: &Library,
749 section: Option<[u8; 16]>,
750 before: &Themes,
751 wrote: Themes,
752) -> Action {
753 let was = Themes {
754 themes: (wrote.themes.iter())
755 .map(|theme| {
756 (before.themes.iter())
757 .find(|kept| kept.id == theme.id)
758 .cloned()
759 .unwrap_or_else(|| Theme {
760 deleted: true,
761 ..theme.clone()
762 })
763 })
764 .collect(),
765 assignments: (wrote.assignments.iter())
766 .map(|assignment| {
767 (before.assignments.iter())
768 .find(|kept| kept.scope == assignment.scope)
769 .cloned()
770 .unwrap_or_else(|| notebook::sidecar::themes::Assignment {
771 theme: None,
772 ..assignment.clone()
773 })
774 })
775 .collect(),
776 };
777 Action {
778 notebook: library.location.clone(),
779 section,
780 change: Change::Themes {
781 from: wrote,
782 to: was,
783 },
784 }
785}
786
704787/// The structure change carrying out a section or group's `change` in `library` now, with
705788/// the action taking it back; none where what it changes is gone or was changed since.
706789fn restructuring(library: &Library, change: &Change) -> Option<(crate::manage::Structure, Action)> {
......@@ -773,6 +856,35 @@ fn restructuring(library: &Library, change: &Change) -> Option<(crate::manage::S
773856 };
774857 (structure, undo)
775858 }
859 Change::Recolor { entry, from, to } => {
860 let (_, path) = entry_of(catalog, *entry)?;
861 if library.section_color(&path) != *from {
862 return None;
863 }
864 (
865 Structure::Color { path, color: *to },
866 Change::Recolor {
867 entry: *entry,
868 from: *to,
869 to: *from,
870 },
871 )
872 }
873 Change::Properties { from, to } => {
874 if library.name != from.0 || from.1.is_some() && library.color() != from.1 {
875 return None;
876 }
877 (
878 Structure::Properties {
879 name: to.0.clone(),
880 color: to.1,
881 },
882 Change::Properties {
883 from: to.clone(),
884 to: from.clone(),
885 },
886 )
887 }
776888 _ => return None,
777889 };
778890 Some((
......@@ -875,6 +987,42 @@ impl State {
875987 section,
876988 change,
877989 } = action;
990 if let Change::Themes { from, to } = change {
991 let library = self
992 .notebooks
993 .iter()
994 .find(|library| library.location == notebook)
995 .cloned()
996 .ok_or("That notebook is closed")?;
997 if !holds(&library.themes(), &from) {
998 return match taken {
999 Some(redo) => self.take(redo),
1000 None => Ok(()),
1001 };
1002 }
1003 let now = crate::filetime();
1004 library.save_themes(Themes {
1005 themes: (to.themes.iter())
1006 .map(|theme| Theme {
1007 modified: now,
1008 ..theme.clone()
1009 })
1010 .collect(),
1011 assignments: (to.assignments.iter())
1012 .map(|assignment| notebook::sidecar::themes::Assignment {
1013 assigned: now,
1014 ..assignment.clone()
1015 })
1016 .collect(),
1017 });
1018 let undo = Action {
1019 notebook,
1020 section,
1021 change: Change::Themes { from: to, to: from },
1022 };
1023 self.undo.note(taken, undo);
1024 return self.wear_theme();
1025 }
8781026 let Some(section) = section else {
8791027 let library = self
8801028 .notebooks
......@@ -1615,6 +1763,127 @@ mod tests {
16151763 }
16161764 }
16171765
1766 /// A section's colour and the notebook's name and colour go back while they are as the
1767 /// change left them.
1768 #[test]
1769 fn colours_and_names_go_back() {
1770 use crate::manage::Structure;
1771 let folder =
1772 std::env::temp_dir().join(format!("snowbound-undo-colours-{}", std::process::id()));
1773 let _ = notebook::fs::remove_dir_all(&folder);
1774 notebook::fs::create_dir_all(&folder).unwrap();
1775 let root = folder.join("Notebook");
1776 let mut notebook =
1777 Notebook::create(&root, folder.join("cache"), Notebook::NEW_COLOR, &dated()).unwrap();
1778 notebook
1779 .set_section_color("New Section 1.one", Some(0x00F0_A0A0))
1780 .unwrap();
1781 let opened = Library::created(root.to_str().unwrap(), notebook, &folder.join("cache"));
1782 let recolor = Structure::Color {
1783 path: "New Section 1.one".into(),
1784 color: Some(0x0000_80FF),
1785 };
1786 let undo = structure_undo(&opened, &recolor).unwrap();
1787 let mut notebook = opened.reopen().unwrap();
1788 notebook
1789 .set_section_color("New Section 1.one", Some(0x0000_80FF))
1790 .unwrap();
1791 let shown = opened.with(notebook);
1792 let (back, redo) = restructuring(&shown, &undo.change).unwrap();
1793 assert!(matches!(
1794 back,
1795 Structure::Color {
1796 color: Some(0x00F0_A0A0),
1797 ..
1798 }
1799 ));
1800 // Coloured again elsewhere, the section keeps that colour.
1801 let mut notebook = shown.reopen().unwrap();
1802 notebook
1803 .set_section_color("New Section 1.one", None)
1804 .unwrap();
1805 assert!(restructuring(&shown.with(notebook), &redo.change).is_none());
1806
1807 let renamed = Structure::Properties {
1808 name: "Renamed".into(),
1809 color: Some(0x0000_FF00),
1810 };
1811 let undo = structure_undo(&shown, &renamed).unwrap();
1812 let Change::Properties { to, .. } = &undo.change else {
1813 panic!()
1814 };
1815 assert_eq!(to, &(shown.name.clone(), Some(Notebook::NEW_COLOR)));
1816 // The name it gave is not the notebook's now, so nothing goes back.
1817 assert!(restructuring(&shown, &undo.change).is_none());
1818 let _ = notebook::fs::remove_dir_all(&folder);
1819 }
1820
1821 /// A theme saved and given to the notebook goes back: a new theme is deleted and the
1822 /// scope's theme is as it was; another device's assignment since leaves it.
1823 #[test]
1824 fn theme_changes_go_back_while_they_hold() {
1825 use notebook::sidecar::themes::{Assignment, Scope, built_in, merge};
1826 let fixture = Fixture::new("themes", &["First"]);
1827 let library = &fixture.library;
1828 let before = library.themes();
1829 let mut mine = built_in().remove(1);
1830 mine.id = "mine".into();
1831 mine.modified = 5;
1832 let wrote = Themes {
1833 themes: vec![mine],
1834 assignments: vec![Assignment {
1835 scope: Scope::Notebook,
1836 theme: Some("mine".into()),
1837 assigned: 5,
1838 }],
1839 };
1840 let mut after = (*before).clone();
1841 merge(&mut after, wrote.clone());
1842 let undo = themes_undo(library, None, &before, wrote);
1843 let Change::Themes { from, to } = &undo.change else {
1844 panic!()
1845 };
1846 assert!(holds(&after, from));
1847 assert!(to.themes[0].deleted);
1848 assert_eq!(to.assignments[0].theme, None);
1849 let mut back = after.clone();
1850 merge(
1851 &mut back,
1852 Themes {
1853 themes: to
1854 .themes
1855 .iter()
1856 .map(|theme| Theme {
1857 modified: 6,
1858 ..theme.clone()
1859 })
1860 .collect(),
1861 assignments: to
1862 .assignments
1863 .iter()
1864 .map(|a| Assignment {
1865 assigned: 6,
1866 ..a.clone()
1867 })
1868 .collect(),
1869 },
1870 );
1871 assert!(holds(&back, to));
1872 assert_eq!(back.effective(None, None), None);
1873 merge(
1874 &mut after,
1875 Themes {
1876 assignments: vec![Assignment {
1877 scope: Scope::Notebook,
1878 theme: Some("editorial".into()),
1879 assigned: 9,
1880 }],
1881 ..Themes::default()
1882 },
1883 );
1884 assert!(!holds(&after, from));
1885 }
1886
16181887 /// Steps a page's editor no longer holds, dropped by a change made elsewhere, leave the
16191888 /// timeline; a page gone takes its steps.
16201889 #[test]