diff --git a/crates/engine/src/cmd/sheet/tabs.rs b/crates/engine/src/cmd/sheet/tabs.rs index a20db5291..2c9425dca 100644 --- a/crates/engine/src/cmd/sheet/tabs.rs +++ b/crates/engine/src/cmd/sheet/tabs.rs @@ -159,6 +159,8 @@ pub(super) fn move_sheet(s: &mut Session, p: &Json) -> Result { let Some(src) = cx.wb.sheets.get(i).cloned() else { return Err(bad("sheet.move", "no such sheet")) }; if copy { let mut sh = (*src).clone(); + // The copy is a sheet of its own, not the one it was copied from. + sh.uid = gridcraft_model::SheetUid::fresh(); let base = sh.name.clone(); // `Name (2)`: the name is shortened so the suffix fits in Excel's 31 characters. let name = (2..) diff --git a/crates/engine/src/tests/seams.rs b/crates/engine/src/tests/seams.rs index 9491c3773..9615257ff 100644 --- a/crates/engine/src/tests/seams.rs +++ b/crates/engine/src/tests/seams.rs @@ -1135,3 +1135,29 @@ fn a_linked_paste_carries_the_files_trust_whatever_it_pastes() { assert!(sheet(&s).cell(b1).is_some_and(|c| c.formula.is_some()), "a linked paste of formats writes a formula"); assert_eq!(trust(&s), Trust::FromFile); } + +#[test] +fn a_sheet_keeps_its_identity_until_it_is_copied_or_read_from_a_file() { + let mut s = s(); + let uids = |s: &Session| s.doc().unwrap().wb.sheets.iter().map(|sh| sh.uid.get()).collect::>(); + let first = uids(&s)[0]; + // Renamed, moved, deleted and brought back by undo: the same sheet. + s.execute("home.insertSheet", json!({"name": "Other"})).unwrap(); + s.execute("sheet.rename", json!({"sheet": "Sheet1", "name": "Data"})).unwrap(); + s.execute("sheet.move", json!({"sheet": "Data", "to": 1})).unwrap(); + let data = s.doc().unwrap().wb.sheet_index("Data").unwrap(); + assert_eq!(uids(&s)[data], first); + s.execute("home.deleteSheet", json!({"sheet": "Data"})).unwrap(); + assert!(!uids(&s).contains(&first)); + s.execute("edit.undo", json!({})).unwrap(); + assert!(uids(&s).contains(&first), "undo brings the same sheet back"); + // A copy, or a new sheet of the same name, is another sheet. + s.execute("sheet.move", json!({"sheet": "Data", "to": 0, "copy": true})).unwrap(); + let all = uids(&s); + assert_eq!(all.len(), 3); + assert_eq!(all.iter().collect::>().len(), 3, "{all:?}"); + // Read from a file: new sheets. + let bytes = crate::io::save_bytes(&s.doc().unwrap().wb, "book.xlsx").unwrap(); + let (read, _) = crate::io::open_bytes("book.xlsx", &bytes).unwrap(); + assert!(read.sheets.iter().all(|sh| !all.contains(&sh.uid.get()))); +} diff --git a/crates/model/src/lib.rs b/crates/model/src/lib.rs index b9b67f519..9df99c3d1 100644 --- a/crates/model/src/lib.rs +++ b/crates/model/src/lib.rs @@ -23,7 +23,7 @@ pub use macros::*; pub use password::PasswordHash; pub use preserved::{Preserved, PreservedPart, PreservedRel, PreservedSheet, PreservedXml}; pub use python::{PyCode, PyOutput}; -pub use sheet::{DEFAULT_COL_WIDTH, DEFAULT_ROW_HEIGHT, LineIndex, Sheet, SheetViewMode, Visibility, points_to_chars}; +pub use sheet::{DEFAULT_COL_WIDTH, DEFAULT_ROW_HEIGHT, LineIndex, Sheet, SheetUid, SheetViewMode, Visibility, points_to_chars}; pub use store::CellStore; pub use style::{ Alignment, BorderLine, BorderStyle, Borders, Color, Fill, Font, HAlign, NumFmt, PatternType, Style, StyleId, StyleTable, Theme, Underline, diff --git a/crates/model/src/sheet.rs b/crates/model/src/sheet.rs index 102a41317..9faebbd5e 100644 --- a/crates/model/src/sheet.rs +++ b/crates/model/src/sheet.rs @@ -2,6 +2,7 @@ use std::collections::BTreeMap; use std::sync::Arc; +use std::sync::atomic::{AtomicU64, Ordering}; use gridcraft_core::{CellRef, RangeRef, Value}; use serde::{Deserialize, Serialize}; @@ -52,9 +53,44 @@ impl SheetViewMode { } } +/// A sheet's identity while the app runs, for what must follow one sheet (an open cell draft): a +/// sheet made new (added, inserted, copied, read from a file or from JSON) gets a fresh one, and it +/// stays with the sheet through renames, moves and the copies undo keeps. Never saved. +/// It is not content: any two compare equal, so comparing sheets compares what they hold; compare +/// identities with [`SheetUid::get`]. +#[derive(Clone, Copy, Debug)] +pub struct SheetUid(u64); + +/// The next [`SheetUid`]; 0 is never given out. +static NEXT_SHEET_UID: AtomicU64 = AtomicU64::new(1); + +impl SheetUid { + pub fn fresh() -> SheetUid { + SheetUid(NEXT_SHEET_UID.fetch_add(1, Ordering::Relaxed)) + } + pub fn get(self) -> u64 { + self.0 + } +} + +impl Default for SheetUid { + fn default() -> SheetUid { + SheetUid::fresh() + } +} + +impl PartialEq for SheetUid { + fn eq(&self, _: &SheetUid) -> bool { + true + } +} + #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct Sheet { pub name: String, + /// The sheet's identity while the app runs ([`SheetUid`]). Not in JSON or the file. + #[serde(skip)] + pub uid: SheetUid, /// The `sheetId` the file gave the sheet, written back on save so that what refers to the /// sheet by its id keeps finding it; 0 for a sheet the file did not have. Not in JSON. #[serde(skip)] @@ -149,6 +185,7 @@ impl Sheet { pub fn new(name: impl Into) -> Sheet { Sheet { name: name.into(), + uid: SheetUid::fresh(), file_id: 0, cells: CellStore::new(), cell_pictures: BTreeMap::new(), @@ -514,6 +551,26 @@ mod tests { use super::*; use crate::cell::Formula; + #[test] + fn a_sheet_keeps_its_identity_through_clones_and_a_new_sheet_gets_its_own() { + let a = Sheet::new("A"); + let b = Sheet::new("A"); + assert_ne!(a.uid.get(), b.uid.get(), "two new sheets"); + assert_ne!(a.uid.get(), 0); + // A clone (an undo snapshot, a rename through Arc::make_mut) is the same sheet. + let mut renamed = a.clone(); + renamed.name = "B".into(); + assert_eq!(renamed.uid.get(), a.uid.get()); + // Not content: equal sheets compare equal whatever their identity. + assert_eq!(a, b); + // Not in JSON: a sheet read back is a new one. + let json = serde_json::to_string(&a).unwrap(); + assert!(!json.contains("uid"), "{json}"); + let read: Sheet = serde_json::from_str(&json).unwrap(); + assert_ne!(read.uid.get(), a.uid.get()); + assert_eq!(read, a); + } + #[test] fn line_index_matches_linear() { let mut s = Sheet::new("S");