Merge PR #3243

crates/calc/src/eval.rs

@@ -359,9 +359,10 @@ impl<'h> Evaluator<'h> {
             return v.clone();
         }
         let wb = self.host.workbook();
-        // Sheet-qualified `Sheet1!Name`.
-        let (scope_sheet, bare) = match n.split_once('!') {
-            Some((s, b)) => match wb.sheet_index(&gridcraft_core::unquote_sheet(s)) {
+        // Sheet-qualified `Sheet1!Name`. The parser stores the sheet unquoted, so it is used as
+        // is (`' Sales '` keeps its spaces) and split at the last `!` (`'a!b'!Name`).
+        let (scope_sheet, bare) = match n.rsplit_once('!') {
+            Some((s, b)) => match wb.sheet_index(s) {
                 Some(i) => (i, b),
                 None => return err(CellError::Name),
             },

crates/chart/src/resolve.rs

@@ -179,9 +179,10 @@ fn category_table(wb: &Workbook, sheet: usize, formula: &str, values: &str) -> (
 /// Whether `t` names a defined name seen from `sheet` (`Name`, or `Sheet1!Name` looked up on
 /// that sheet), the way the evaluator looks names up.
 fn is_defined_name(wb: &Workbook, sheet: usize, t: &str) -> bool {
-    match t.split_once('!') {
-        Some((s, bare)) => wb.sheet_index(s.trim_matches('\'')).is_some_and(|i| wb.name(bare, i).is_some()),
-        None => wb.name(t, sheet).is_some(),
+    // `'Bob''s'!Name`: the sheet is unquoted as everywhere else (#2012).
+    match gridcraft_core::split_sheet(t) {
+        (Some(s), bare) => wb.sheet_index(&s).is_some_and(|i| wb.name(bare, i).is_some()),
+        (None, _) => wb.name(t, sheet).is_some(),
     }
 }
 

crates/chart/src/tests.rs

@@ -1258,3 +1258,75 @@ fn box_whisker_draws_each_series_quartile_method() {
     // The boxes span 3.5 and 2.5 data units on the same value axis.
     assert!((heights[0] / heights[1] - 1.4).abs() < 0.001);
 }
+
+#[test]
+fn a_series_named_by_a_name_on_a_sheet_with_an_apostrophe_resolves() {
+    // #2012: `'Bob''s'!Title` kept the doubled quote, so the sheet wasn't found and the series
+    // was named by the literal text.
+    let mut wb = Workbook::new();
+    {
+        let s = wb.sheet_mut(0).unwrap();
+        s.name = "Bob's".into();
+        s.set_value(CellRef::new(0, 0), Value::from("Takings"));
+        s.set_value(CellRef::new(1, 0), Value::Number(4.0));
+        s.set_value(CellRef::new(2, 0), Value::Number(9.0));
+    }
+    wb.names.push(gridcraft_model::DefinedName {
+        name: "Title".into(),
+        scope: Some(0),
+        formula: "'Bob''s'!$A$1".into(),
+        comment: String::new(),
+        hidden: false,
+    });
+    let mut c = chart(ChartKind::ColumnClustered);
+    c.series = vec![Series {
+        name: Some("'Bob''s'!Title".into()),
+        categories: None,
+        values: "'Bob''s'!$A$2:$A$3".into(),
+        bubble_sizes: None,
+        color: None,
+        secondary: false,
+        kind: None,
+        quartiles: Default::default(),
+        smooth: false,
+        trendlines: Vec::new(),
+    }];
+    let d = resolve(&wb, 0, &c);
+    assert_eq!(d.series.first().map(|s| s.name.as_str()), Some("Takings"));
+}
+
+#[test]
+fn a_series_named_by_a_name_on_a_sheet_with_a_bang_resolves() {
+    // `'a!b'!Title`: the sheet name holds a `!`, so the evaluator must split the parsed name at
+    // the last one, not the first.
+    let mut wb = Workbook::new();
+    {
+        let s = wb.sheet_mut(0).unwrap();
+        s.name = "a!b".into();
+        s.set_value(CellRef::new(0, 0), Value::from("Takings"));
+        s.set_value(CellRef::new(1, 0), Value::Number(4.0));
+        s.set_value(CellRef::new(2, 0), Value::Number(9.0));
+    }
+    wb.names.push(gridcraft_model::DefinedName {
+        name: "Title".into(),
+        scope: Some(0),
+        formula: "'a!b'!$A$1".into(),
+        comment: String::new(),
+        hidden: false,
+    });
+    let mut c = chart(ChartKind::ColumnClustered);
+    c.series = vec![Series {
+        name: Some("'a!b'!Title".into()),
+        categories: None,
+        values: "'a!b'!$A$2:$A$3".into(),
+        bubble_sizes: None,
+        color: None,
+        secondary: false,
+        kind: None,
+        quartiles: Default::default(),
+        smooth: false,
+        trendlines: Vec::new(),
+    }];
+    let d = resolve(&wb, 0, &c);
+    assert_eq!(d.series.first().map(|s| s.name.as_str()), Some("Takings"));
+}

crates/ui-egui/src/macro_editor.rs

@@ -459,10 +459,7 @@ pub fn parse_refs(text: &str) -> Vec<(Option<String>, RangeRef)> {
     text.split(',')
         .filter_map(|part| {
             let part = part.trim();
-            let (sheet, body) = match part.rsplit_once('!') {
-                Some((s, b)) => (Some(s.trim().trim_matches('\'').replace("''", "'")), b),
-                None => (None, part),
-            };
+            let (sheet, body) = gridcraft_engine::core::split_sheet(part);
             RangeRef::parse(&body.replace('$', "")).map(|r| (sheet, r))
         })
         .take(500)
@@ -2399,6 +2396,15 @@ pub fn paint_overlay(p: &egui::Painter, geo: &crate::grid::Geo, sh: &gridcraft_e
 mod tests {
     use super::*;
 
+    #[test]
+    fn example_references_unquote_sheet_names_as_formulas_do() {
+        // #2012: one shared reading of `'Bob''s'!A1` and `'a!b'!A1`.
+        let refs = parse_refs("'Bob''s'!$A$1:$B$2, 'a!b'!C3, D4");
+        let sheets: Vec<Option<String>> = refs.iter().map(|(s, _)| s.clone()).collect();
+        assert_eq!(sheets, [Some("Bob's".to_string()), Some("a!b".to_string()), None]);
+        assert_eq!(refs.iter().map(|(_, r)| r.a1()).collect::<Vec<_>>(), ["A1:B2", "C3", "D4"]);
+    }
+
     #[test]
     fn an_edited_input_keeps_a_shape_a_macro_may_declare() {
         let range = NamedInput { rows: [2, 10], cols: [3, 3], header: true, col_types: vec![ColType::Number; 3], ..NamedInput::default() };

crates/xlsx/src/chart.rs

@@ -366,9 +366,11 @@ fn is_ref(f: &str) -> bool {
 /// when it isn't a name, or names a sheet the workbook doesn't have.
 fn chart_name<'a>(wb: &'a Workbook, si: usize, f: &str) -> Option<(Option<usize>, String, Option<&'a DefinedName>)> {
     let Ok(Expr::Name(n)) = gridcraft_formula::parse(f.trim()) else { return None };
-    match n.split_once('!') {
+    // The parser has already unquoted the sheet (`'Bob''s'!Name` → `Bob's!Name`, #2012): split
+    // at the last `!` (`'a!b'!Name`) and keep the name as is (`' Sales '` keeps its spaces).
+    match n.rsplit_once('!') {
         Some((s, bare)) => {
-            let i = wb.sheet_index(s.trim_matches('\''))?;
+            let i = wb.sheet_index(s)?;
             Some((Some(i), bare.to_string(), wb.name(bare, i)))
         }
         None => {

crates/xlsx/src/tests/chart_book_names.rs

@@ -139,3 +139,41 @@ fn chart_names_are_written_qualified() {
     let refs = |x: &str| x.split("<c:f>").skip(1).filter_map(|p| p.split_once("</c:f>").map(|(f, _)| f.to_string())).collect::<Vec<_>>();
     assert_eq!(refs(&again), refs(&xml));
 }
+
+/// A quoted sheet name keeps its outer spaces: `' Sales '!Data` is not `Sales!Data`, so the
+/// chart doesn't switch to the other sheet's name on save.
+#[test]
+fn chart_names_keep_spaces_around_a_quoted_sheet_name() {
+    let mut wb = Workbook::default();
+    let mut s = Sheet::new("Sales");
+    s.charts.push(Chart {
+        id: 1,
+        kind: ChartKind::BarStacked,
+        anchor: Anchor { cell: gridcraft_core::CellRef::new(0, 3), width: 480.0, height: 300.0, ..Anchor::default() },
+        title: None,
+        series: vec![series("' Sales '!Label", "' Sales '!Data")],
+        legend: LegendPos::Bottom,
+        data_labels: false,
+        data_table: None,
+        gridlines: true,
+        style: 2,
+        x_title: None,
+        y_title: None,
+        source: None,
+        by_rows: false,
+        pivot: None,
+    });
+    wb.sheets = vec![Arc::new(s), Arc::new(Sheet::new(" Sales "))];
+    wb.names = vec![
+        name("Items", None, "Sales!$A$2:$A$4"),
+        name("Data", Some(0), "Sales!$B$2:$B$4"),
+        name("Data", Some(1), "' Sales '!$B$2:$B$4"),
+        name("Label", Some(0), "Sales!$B$1"),
+        name("Label", Some(1), "' Sales '!$B$1"),
+    ];
+    let bytes = write_xlsx(&wb).unwrap();
+    let mut xml = String::new();
+    zip::ZipArchive::new(std::io::Cursor::new(&bytes)).unwrap().by_name("xl/charts/chart1.xml").unwrap().read_to_string(&mut xml).unwrap();
+    assert!(xml.contains("<c:val><c:numRef><c:f>' Sales '!Data</c:f>"), "{xml}");
+    assert!(xml.contains("<c:tx><c:strRef><c:f>' Sales '!Label</c:f>"), "{xml}");
+}