Skip to content

HF-160: ADDRESS drops the sheet separator when sheetName is empty - #1739

Open
marcin-kordas-hoc wants to merge 2 commits into
developfrom
spike/hf160-address-sheetname
Open

HF-160: ADDRESS drops the sheet separator when sheetName is empty#1739
marcin-kordas-hoc wants to merge 2 commits into
developfrom
spike/hf160-address-sheetname

Conversation

@marcin-kordas-hoc

@marcin-kordas-hoc marcin-kordas-hoc commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Fixes HF-160 (GH #1641): =ADDRESS(2,3,1,FALSE(),) returned !R2C3 — the sheet separator was emitted for an empty-string sheet name. Excel returns R2C3.

What changed

One condition in AddressPlugin: an empty string no longer produces a sheet prefix, for both A1 and R1C1 styles. Non-empty sheet names keep producing Sheet1!R2C3 exactly as before.

Verification

Paired tests: hyperformula-tests@spike/hf160-address-sheetname (37 assertions: trailing-comma / explicit "" / omitted forms in both styles, non-empty names pinned unchanged). Full unit/interpreter 390 suites / 3531 tests green, tsc --noEmit clean, eslint --quiet clean.

🤖 Generated with Claude Code


Note

Low Risk
Single-condition change in ADDRESS string formatting; no auth, data, or broad API impact.

Overview
Fixes HF-160: ADDRESS no longer emits a stray ! when the optional sheet name is an empty string (e.g. trailing comma or explicit ""), so R1C1 results match Excel (R2C3 instead of !R2C3).

The only code change is in AddressPlugin: the sheet prefix is built only when sheetName is defined, non-null, and non-empty. Non-empty names still produce Sheet1!… as before.

Reviewed by Cursor Bugbot for commit deb5b76. Bugbot is set up for automated code reviews on this repo. Configure here.

=ADDRESS(2,3,1,FALSE(),) returned '!R2C3' - the '!' separator was emitted
for an empty-string sheet name. Excel returns 'R2C3'. One condition: an
empty string no longer produces a sheet prefix, for both A1 and R1C1
styles; non-empty sheet names are untouched.

Implemented by a prep-ship lane (task HF-160); the loop could not see its
own green (harness path bug, prep-ship#1 follow-up) so verified here:
authored spec 37/37, unit/interpreter 390 suites / 3531 tests green,
tsc --noEmit clean, eslint clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PdHPZAjciZFWqGa19Yf7it
@qunabu

qunabu commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 18, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
hyperformula-docs 62cb944 Commit Preview URL

Branch Preview URL
Aug 20 2026, 09:04 AM

@github-actions

github-actions Bot commented Aug 18, 2026

Copy link
Copy Markdown

Performance comparison of head (62cb944) vs base (61ead73)

                                     testName |    base |    head | change
--------------------------------------------------------------------------
                                      Sheet A |  500.56 |  497.53 | -0.61%
                                      Sheet B |  162.33 |  163.48 | +0.71%
                                      Sheet T |  145.27 |  141.92 | -2.31%
                                Column ranges |  479.58 |  476.99 | -0.54%
                                Sorted lookup | 14657.9 | 14181.8 | -3.25%
Sheet A:  change value, add/remove row/column |   17.36 |   16.34 | -5.88%
 Sheet B: change value, add/remove row/column |  134.42 |     145 | +7.87%
                   Column ranges - add column |  152.65 |  156.51 | +2.53%
                Column ranges - without batch |  460.02 |  474.98 | +3.25%
                        Column ranges - batch |  120.85 |  121.08 | +0.19%

@marcin-kordas-hoc
marcin-kordas-hoc marked this pull request as ready for review August 18, 2026 05:49
@marcin-kordas-hoc

Copy link
Copy Markdown
Collaborator Author

Paired tests PR: handsontable/hyperformula-tests#40 — merge it BEFORE this one.

@codecov

codecov Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.31%. Comparing base (61ead73) to head (62cb944).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1739   +/-   ##
========================================
  Coverage    97.31%   97.31%           
========================================
  Files          195      195           
  Lines        15719    15719           
  Branches      3455     3384   -71     
========================================
  Hits         15297    15297           
- Misses         414      422    +8     
+ Partials         8        0    -8     
Files with missing lines Coverage Δ
src/interpreter/plugin/AddressPlugin.ts 100.00% <100.00%> (ø)

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants