From c5de19a287c3e7c643fa1eacca0ff98de6f0cc45 Mon Sep 17 00:00:00 2001 From: Divyanshu Gupta Date: Thu, 22 Oct 2026 01:01:54 +0530 Subject: [PATCH] fix(engine, layout): invalidate layout cache on filename change and unify untitled fallback --- crates/engine/src/cmd/fields.rs | 18 ++------ crates/engine/src/cmd/references.rs | 41 +++++++++++++----- crates/engine/src/session.rs | 38 +++++++++++------ crates/layout/src/lib.rs | 30 +++++++------- crates/layout/src/tests.rs | 64 +++++++++++++++++++++++++++++ 5 files changed, 140 insertions(+), 51 deletions(-) diff --git a/crates/engine/src/cmd/fields.rs b/crates/engine/src/cmd/fields.rs index 371538129..c390a8215 100644 --- a/crates/engine/src/cmd/fields.rs +++ b/crates/engine/src/cmd/fields.rs @@ -193,6 +193,7 @@ fn shown_text(s: &mut Session, f: &FieldAt) -> String { .map(|p| l.pages.iter().filter(|pg| pg.section == p.section).count() as u32) .unwrap_or_else(|| u32::try_from(l.pages.len()).unwrap_or(u32::MAX)); let page_format = page.and_then(|p| s.doc.sections().get(p.section).map(|(_, sec)| sec.page_num_format)).unwrap_or_default(); + let (fn_name, fn_path) = s.resolved_filenames(); let ctx = wordcraft_layout::fields::FieldCtx { page: page.map(|p| p.number).unwrap_or(1), page_format, @@ -201,21 +202,8 @@ fn shown_text(s: &mut Session, f: &FieldAt) -> String { section, title: s.doc.core.title.as_str().into(), author: s.doc.core.creator.as_str().into(), - filename: s - .path - .as_ref() - .and_then(|p| p.file_name()) - .map(|n| n.to_string_lossy().to_string()) - .or_else(|| s.file_name.clone()) - .unwrap_or_else(|| "Document1".into()) - .into(), - filepath: s - .path - .as_ref() - .map(|p| p.to_string_lossy().to_string()) - .or_else(|| s.file_name.clone()) - .unwrap_or_else(|| "Document1".into()) - .into(), + filename: fn_name.into(), + filepath: fn_path.into(), ..Default::default() }; wordcraft_layout::fields::field_text(&instr, &result, &ctx).0 diff --git a/crates/engine/src/cmd/references.rs b/crates/engine/src/cmd/references.rs index eb8d2159c..f1a311994 100644 --- a/crates/engine/src/cmd/references.rs +++ b/crates/engine/src/cmd/references.rs @@ -1014,16 +1014,8 @@ pub fn update_fields_in(s: &mut Session, all: bool) -> Result<(), CmdError> { "AUTHOR" => updates.push((k, s.doc.core.creator.clone())), "NUMWORDS" => updates.push((k, s.doc.word_count().to_string())), "FILENAME" => { - let text = if has_switch(instr, 'p') { - s.path.as_ref().map(|p| p.to_string_lossy().to_string()).or_else(|| s.file_name.clone()).unwrap_or_else(|| "Document1".into()) - } else { - s.path - .as_ref() - .and_then(|p| p.file_name()) - .map(|n| n.to_string_lossy().to_string()) - .or_else(|| s.file_name.clone()) - .unwrap_or_else(|| "Document1".into()) - }; + let (fn_name, fn_path) = s.resolved_filenames(); + let text = if has_switch(instr, 'p') { fn_path } else { fn_name }; updates.push((k, text)); } _ => {} @@ -1351,6 +1343,35 @@ mod tests { assert_eq!(fields[1], (r"FILENAME \p".into(), "/docs/my_thesis.docx".into())); } + #[test] + fn update_fields_filename_titled_unsaved() { + let mut doc = wordcraft_doc::Document::new(); + doc.core.title = "Important Document".into(); + let mut s = Session::new(doc); + s.run("insert.field", &serde_json::json!({"instr": "FILENAME"})).unwrap(); + let fields = story_fields(&s, StoryRef::Body); + assert_eq!(fields[0], ("FILENAME".into(), "Document1".into())); + + let l = s.layout(); + let text: String = l.pages[0] + .items + .iter() + .filter_map(|it| { + if let wordcraft_layout::Placed::Lines { para, .. } = it { + Some(para.shown.iter().map(|x| x.1.clone()).collect::()) + } else { + None + } + }) + .collect(); + assert!(text.contains("Document1"), "live layout renders Document1: {text}"); + assert!(!text.contains("Important Document"), "live layout must not render title: {text}"); + + s.run("references.updateFields", &serde_json::json!({})).unwrap(); + let fields_after = story_fields(&s, StoryRef::Body); + assert_eq!(fields_after[0], ("FILENAME".into(), "Document1".into())); + } + /// #1706 (Word check): Update Field with the caret in the body leaves a header's fields alone. #[test] fn update_fields_in_the_body_leaves_headers_alone() { diff --git a/crates/engine/src/session.rs b/crates/engine/src/session.rs index 3c48e6a17..8b3638b3e 100644 --- a/crates/engine/src/session.rs +++ b/crates/engine/src/session.rs @@ -803,6 +803,22 @@ impl Session { if set.is_empty() { self.author.split_whitespace().filter_map(|w| w.chars().next()).collect() } else { set.to_string() } } + /// The document's file name and full path for fields (`FILENAME`) and layout context. + /// Returns `(filename, filepath)`. Unsaved documents with no explicit file name default + /// consistently to `("Document1", "Document1")`. Document metadata title is never used + /// as a filename fallback (#3449). + pub fn resolved_filenames(&self) -> (String, String) { + if let Some(path) = &self.path { + let name = path.file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| path.to_string_lossy().to_string()); + let full = path.to_string_lossy().to_string(); + (name, full) + } else if let Some(name) = &self.file_name { + (name.clone(), name.clone()) + } else { + ("Document1".to_string(), "Document1".to_string()) + } + } + /// The current layout (recomputed when the document or view changed). pub fn layout(&mut self) -> Arc { let mode = if self.view.read_mode { ViewMode::Read } else { self.view.mode }; @@ -833,17 +849,8 @@ impl Session { { return l.clone(); } - let (filename, filepath): (Option>, Option>) = if let Some(path) = &self.path { - let name = path.file_name().map(|n| n.to_string_lossy().to_string()).unwrap_or_else(|| path.to_string_lossy().to_string()); - let full = path.to_string_lossy().to_string(); - (Some(Arc::from(name.as_str())), Some(Arc::from(full.as_str()))) - } else if let Some(name) = &self.file_name { - (Some(Arc::from(name.as_str())), Some(Arc::from(name.as_str()))) - } else if !self.doc.core.title.is_empty() { - (Some(Arc::from(self.doc.core.title.as_str())), Some(Arc::from(self.doc.core.title.as_str()))) - } else { - (Some(Arc::from("Document1")), Some(Arc::from("Document1"))) - }; + let (fn_name, fn_path) = self.resolved_filenames(); + let (filename, filepath): (Option>, Option>) = (Some(Arc::from(fn_name.as_str())), Some(Arc::from(fn_path.as_str()))); let opts = LayoutOptions { view: mode, web_width: width, @@ -912,7 +919,14 @@ impl Session { /// [`Session::export_layout`]; with `markup` (Print Markup) each page also has the markup /// area with the comments in balloons. pub fn export_layout_with(&self, markup: bool) -> Arc { - let opts = LayoutOptions { view: ViewMode::Print, comment_balloons: markup && self.prefs.markup.comments, ..Default::default() }; + let (fn_name, fn_path) = self.resolved_filenames(); + let opts = LayoutOptions { + view: ViewMode::Print, + comment_balloons: markup && self.prefs.markup.comments, + filename: Some(Arc::from(fn_name.as_str())), + filepath: Some(Arc::from(fn_path.as_str())), + ..Default::default() + }; Arc::new(wordcraft_layout::layout(&self.doc, &mut LayoutCache::new(), &opts)) } diff --git a/crates/layout/src/lib.rs b/crates/layout/src/lib.rs index 4f2c27fc8..f05ec80eb 100644 --- a/crates/layout/src/lib.rs +++ b/crates/layout/src/lib.rs @@ -587,20 +587,22 @@ fn env_hash(doc: &Document, opts: &LayoutOptions) -> u64 { let s = serde_json::to_string(&(&doc.styles, &doc.numbering, doc.settings.default_tab, &doc.settings.footnote_format)).unwrap_or_default(); // Outline view lays paragraphs out changed (no indents; plain without text formatting). let outline = (opts.view == ViewMode::Outline).then(|| opts.outline.as_ref().is_some_and(|o| o.plain)); - hash_of(&( - s, - opts.show_hidden, - opts.hide_deleted, - opts.hide_marked, - opts.deletions_in_balloons, - opts.proofing, - opts.numeral, - opts.field_codes, - opts.proof, - wordcraft_proof::dictionary_generation(), - hyphenation_key(doc), - outline, - )) + let mut h = std::collections::hash_map::DefaultHasher::new(); + s.hash(&mut h); + opts.show_hidden.hash(&mut h); + opts.hide_deleted.hash(&mut h); + opts.hide_marked.hash(&mut h); + opts.deletions_in_balloons.hash(&mut h); + opts.proofing.hash(&mut h); + opts.numeral.hash(&mut h); + opts.field_codes.hash(&mut h); + opts.proof.hash(&mut h); + wordcraft_proof::dictionary_generation().hash(&mut h); + hyphenation_key(doc).hash(&mut h); + outline.hash(&mut h); + opts.filename.hash(&mut h); + opts.filepath.hash(&mut h); + h.finish() } /// The hyphenation settings, hashable (the zone as its bits). diff --git a/crates/layout/src/tests.rs b/crates/layout/src/tests.rs index 174dde174..bfc405a50 100644 --- a/crates/layout/src/tests.rs +++ b/crates/layout/src/tests.rs @@ -9423,3 +9423,67 @@ fn caret_height_ignores_line_spacing() { assert!((single - wide).abs() < 0.01, "{spacing:?}: single {single}, wide {wide}"); } } + +#[test] +fn layout_cache_invalidates_on_changed_filename() { + let mut doc = Document::new(); + let mut p = Paragraph::new(); + p.insert_object( + 0, + InlineObject::Field { instr: "FILENAME".into(), result: String::new(), locked: false, dirty: false, code: false, form: String::new() }, + &Default::default(), + ) + .unwrap(); + doc.body = vec![wordcraft_doc::para_block(p)]; + + let shown = |l: &DocLayout| -> String { + l.pages[0] + .items + .iter() + .find_map(|it| if let Placed::Lines { para, .. } = it { para.shown.first().map(|s| s.1.clone()) } else { None }) + .unwrap_or_default() + }; + + let mut cache = LayoutCache::new(); + let opts_a = LayoutOptions { filename: Some(Arc::from("a.docx")), filepath: Some(Arc::from("/docs/a.docx")), ..Default::default() }; + let l_a = layout(&doc, &mut cache, &opts_a); + let text_a = shown(&l_a); + assert_eq!(text_a, "a.docx", "first layout shows a.docx"); + + let opts_b = LayoutOptions { filename: Some(Arc::from("b.docx")), filepath: Some(Arc::from("/docs/b.docx")), ..Default::default() }; + let l_b = layout(&doc, &mut cache, &opts_b); + let text_b = shown(&l_b); + assert_eq!(text_b, "b.docx", "second layout with same cache shows b.docx"); +} + +#[test] +fn layout_cache_invalidates_on_changed_filepath() { + let mut doc = Document::new(); + let mut p = Paragraph::new(); + p.insert_object( + 0, + InlineObject::Field { instr: r"FILENAME \p".into(), result: String::new(), locked: false, dirty: false, code: false, form: String::new() }, + &Default::default(), + ) + .unwrap(); + doc.body = vec![wordcraft_doc::para_block(p)]; + + let shown = |l: &DocLayout| -> String { + l.pages[0] + .items + .iter() + .find_map(|it| if let Placed::Lines { para, .. } = it { para.shown.first().map(|s| s.1.clone()) } else { None }) + .unwrap_or_default() + }; + + let mut cache = LayoutCache::new(); + let opts_a = LayoutOptions { filename: Some(Arc::from("a.docx")), filepath: Some(Arc::from("/path/one/a.docx")), ..Default::default() }; + let l_a = layout(&doc, &mut cache, &opts_a); + let text_a = shown(&l_a); + assert_eq!(text_a, "/path/one/a.docx", "first layout shows /path/one/a.docx"); + + let opts_b = LayoutOptions { filename: Some(Arc::from("a.docx")), filepath: Some(Arc::from("/path/two/a.docx")), ..Default::default() }; + let l_b = layout(&doc, &mut cache, &opts_b); + let text_b = shown(&l_b); + assert_eq!(text_b, "/path/two/a.docx", "second layout with same cache shows /path/two/a.docx"); +}