[Aryan007-dev]

Closes #587

buildNamedRanges built _xlnm.Print_Area and _xlnm.Print_Titles as ${sheet.name}!range with no quoting. For a sheet such as Detail Sheet, the workbook got Detail Sheet!$1:$4, which Excel opens without complaint but ignores as a print setting.

The chart series writer already had a quoteSheetName helper that quotes names when needed and doubles embedded '. I moved it unchanged to src/_refs.ts next to unquoteSheet and used it for the derived print ranges as well, so both writers quote the same way. A comment at the call site references #587.

Output for the issue's repro:

- <definedName name="_xlnm.Print_Titles" localSheetId="0">Detail Sheet!$1:$4</definedName>
+ <definedName name="_xlnm.Print_Titles" localSheetId="0">'Detail Sheet'!$1:$4</definedName>

Why the tests didn't catch it

The existing print-range tests only use sheet names that need no quoting (Report, Sheet1). The reader strips everything up to the last !, so the round trip passed either way.

Tests

Added to test/named-ranges.test.ts:

• quotes sheet names that need it in print area and titles (Print_Titles / Print_Area written with an unquoted sheet name — Excel drops them for names with spaces #587): covers Detail Sheet (area, row titles and column titles) and O'Brien ('O''Brien'!$1:$1). This one fails against main with Detail Sheet!$A$1:$B$4.
• reads quoted print ranges back into pageSetup (Print_Titles / Print_Area written with an unquoted sheet name — Excel drops them for names with spaces #587): a round-trip guard. It also passes on main, because the reader already handles quoted names. It is there so the write and read sides stay in step now that the writer quotes.

Local bun run test (Bun 1.4.2, Node 24):

• oxlint, oxfmt --check and both tsc --noEmit runs are clean.
• vitest: 10693/10694 passed. The one failure is html-import.test.ts › "refuses a table whose colspans describe more cells than a sheet holds", which hits its 20s timeout on my machine during the full parallel run and passes when run on its own. That test doesn't touch this change.

Not verified

• I did not open the output in Excel. The quoting is the form the issue reports as working in Excel for Mac, and the same rule the chart writer already uses for <c:f> references.
• Sheet names that look like cell references (e.g. A1) or start with a digit-like token are quoted only if they fail ^[A-Za-z_][A-Za-z0-9_]*$, which is the existing helper's rule. I didn't change that rule here.

Summary by CodeRabbit

• Bug Fixes
  • Chart references now correctly handle worksheet names that require quoting, including names with spaces or apostrophes.
  • Print-area and print-title references now use correctly quoted worksheet names, preserving embedded apostrophes.
  • Quoted print ranges for worksheets with spaces are restored correctly when opening a workbook and are not listed as named ranges.

[coderabbitai]

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: e537c42a-6aa5-4e6b-86b5-17f6510390dd

📥 Commits

Reviewing files that changed from the base of the PR and between bff162f and 6a20d1c.

📒 Files selected for processing: 3

• src/xlsx/chart/series.ts
• src/xlsx/derived-ranges.ts
• src/xlsx/sheet-name.ts

 _________________________________________________________________________________________________________
< A good programmer is someone who always looks both ways before crossing a one-way street. - Doug Linder >
 ---------------------------------------------------------------------------------------------------------
  \
   \   (\__/)
       (•ㅅ•)
       / 　 づ

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: 9b4d55b9-fcc1-465a-a7bf-a8130da47b06

📥 Commits

Reviewing files that changed from the base of the PR and between d5fbbcf and bff162f.

📒 Files selected for processing (4)

• src/_refs.ts
• src/xlsx/chart/series.ts
• src/xlsx/derived-ranges.ts
• test/named-ranges.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.

📝 Walkthrough

The change adds a shared sheet-name quoting helper. Chart references and generated print-area and print-title names use the helper. Tests cover names with spaces and embedded apostrophes, plus round-trip handling of print ranges.

Changes: Sheet Reference Quoting

Layer / File(s)	Summary
Shared sheet-name quoting src/_refs.ts, src/xlsx/chart/series.ts	Adds an exported helper that leaves plain identifiers unquoted and quotes other sheet names, doubling embedded apostrophes. Chart reference qualification uses the shared helper for bare ranges.
Print-range defined names src/xlsx/derived-ranges.ts, test/named-ranges.test.ts	Generated print-area and print-title formulas use quoted sheet names. Tests cover writing names with spaces and apostrophes, and restoring print settings from quoted ranges.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: productdevbook

Merge Risk: ⚪ Minimal · up to bff16

Generated print settings preserve quoted sheet names, including names with apostrophes, when read back. No actionable merge risk remains; proceed after normal checks.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The change satisfies issue #587. buildNamedRanges now applies quoteSheetName to printArea, printTitlesRow, and printTitlesColumn. The helper leaves plain identifiers unchanged, quotes names …
Out of Scope Changes check	✅ Passed	The changes stay within issue #587. Moving the existing helper to src/_refs.ts supports shared quoting behavior for chart references and derived print ranges. The added tests verify the requested wr…
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 4 files.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: quoting sheet names in generated print-area and print-title references.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[productdevbook]

Taking this with one commit on top, and the diagnosis in the description was right — the reader strips to the last !, so the round trip could never see the missing quotes.

The one problem was where the shared helper landed. _refs.ts imports cell-utils' formula walker, so chart/series.ts importing quoteSheetName from there pulled that graph into every bundle the chart reader is part of: +1.1 KB gzipped on readXlsx and +0.8 KB on writeXlsx, which is what turned coverage + size red (two scenarios over scripts/size-budget.json).

The three lines now live in src/xlsx/sheet-name.ts, a leaf module, so the chart writer and the print defined names still quote from one implementation. Measured against main with the branch merged in: readXlsx 37.5 KB and writeXlsx 37.9 KB gzipped — identical to baseline, all within budget. Your fix and both tests are unchanged.