mirror of
https://github.com/storytold/gridcraft.git
synced 2026-11-03 17:21:56 +00:00
#2012: split a parsed sheet-qualified name at the last !, sheet as is
Addresses review comments 4455115540 and 4455115557. The parser stores `'Sheet'!Name` as `Sheet!Name` with the sheet already unquoted: - Evaluator::name split it at the first `!`, so a series named `'a!b'!Title` showed #NAME?. - chart_name unquoted it again, trimming spaces, so a chart reference to `' Sales '!Data` was saved as `Sales!Data`. Both now split at the last `!` and look the sheet up as is. Tests: chart tests::a_series_named_by_a_name_on_a_sheet_with_a_bang_resolves, xlsx tests::chart_book_names::chart_names_keep_spaces_around_a_quoted_sheet_name. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
e89ba799e9
commit
2caf435a38
4 files changed
+84
-8
No files matched your search
@@ -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),
|
||||
},
|
||||
|
||||
@@ -1294,3 +1294,39 @@ fn a_series_named_by_a_name_on_a_sheet_with_an_apostrophe_resolves() {
|
||||
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"));
|
||||
}
|
||||
@@ -366,13 +366,14 @@ 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 };
|
||||
// `'Bob''s'!Name`: the sheet is unquoted as everywhere else (#2012).
|
||||
match gridcraft_core::split_sheet(&n) {
|
||||
(Some(s), bare) => {
|
||||
let i = wb.sheet_index(&s)?;
|
||||
// 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)?;
|
||||
Some((Some(i), bare.to_string(), wb.name(bare, i)))
|
||||
}
|
||||
(None, _) => {
|
||||
None => {
|
||||
let def = wb.name(&n, si);
|
||||
Some((None, n, def))
|
||||
}
|
||||
|
||||
@@ -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}");
|
||||
}
|
||||
Reference in new issue
Block a user