Skip to content

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

Open
marcin-kordas-hoc wants to merge 4 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 4 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
Behavior change is scoped behind the new emptyAsAbsent flag; only ADDRESS adopts it, with limited impact on the shared argument-coercion path.

Overview
ADDRESS now matches Excel for the optional sheet name: omitted or trailing-comma empty slots no longer add the ! prefix (e.g. R2C3 instead of !R2C3), while an explicit empty value ("" or an empty cell) still keeps !.

This is enabled by a new emptyAsAbsent parameter flag on custom/built-in functions. In FunctionPlugin, syntactically empty argument slots can be passed through as undefined instead of type zero-values ("", 0, FALSE), only when emptyAsDefault is not in play. ADDRESS’s fifth argument uses emptyAsAbsent: true.

Docs and changelog describe the new option and the ADDRESS fix.

Reviewed by Cursor Bugbot for commit dbab806. 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 dbab806 Commit Preview URL

Branch Preview URL
Aug 31 2026, 07:14 AM

@github-actions

github-actions Bot commented Aug 18, 2026

Copy link
Copy Markdown

Performance comparison of head (dbab806) vs base (114fd5d)

                                     testName |    base |    head |  change
---------------------------------------------------------------------------
                                      Sheet A |  492.09 |  516.99 |  +5.06%
                                      Sheet B |  158.71 |  175.08 | +10.31%
                                      Sheet T |  137.73 |  147.66 |  +7.21%
                                Column ranges |  515.08 |  545.26 |  +5.86%
                                Sorted lookup | 14980.3 | 16553.5 | +10.50%
Sheet A:  change value, add/remove row/column |    19.7 |   18.11 |  -8.07%
 Sheet B: change value, add/remove row/column |  185.46 |  162.49 | -12.39%
                   Column ranges - add column |  186.77 |  180.46 |  -3.38%
                Column ranges - without batch |   561.4 |  551.33 |  -1.79%
                        Column ranges - batch |  140.52 |  137.96 |  -1.82%

@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.

marcin-kordas-hoc and others added 3 commits August 20, 2026 09:01
… when absent

Measured in Excel via the Graph API (three independent sessions, LEN()-proven),
which contradicts this task's acceptance criteria:

  =ADDRESS(2,3,1,FALSE)        -> R2C3   (LEN 4, no "!")
  =ADDRESS(2,3,1,FALSE,)       -> R2C3   (LEN 4, no "!")
  =ADDRESS(2,3,1,FALSE,"")     -> !R2C3  (LEN 5, keeps "!")
  =ADDRESS(2,3,1,FALSE,<empty cell ref>) -> !R2C3 (LEN 5, keeps "!")
  =ADDRESS(1,1,4,TRUE,"")      -> !A1    (LEN 3, keeps "!")

Excel distinguishes an argument that is syntactically absent from one that is
present with an empty value. The AC asked for the separator to be dropped in all
three empty-ish cases "consistently with Excel"; that premise holds for only one
of them. The previous revision of this branch implemented the AC literally and
regressed three cases that develop already got right, so this replaces it.

The remaining real defect is the empty argument slot: a trailing comma reached the
implementation as "" and produced a stray "!". Rather than hand-inspecting
AstNodeType.EMPTY in the plugin, this adds an `emptyAsAbsent` argument-validation
option next to the existing `emptyAsDefault`, so the distinction lives in the
argument metadata and every function can opt in. ADDRESS's sheetName uses it.

`emptyAsAbsent` is part of the custom-function metadata surface, so it is
documented in docs/guide/custom-functions.md and the CHANGELOG.

Verified: 6185/6189 jest tests pass. The single failure (=MOD(-7,2) in
unit/function-metadata-api.spec.ts) reproduces on clean develop with clean
develop tests and is unrelated: it is a stale golden expectation left by the
MOD sign fix in #1752.
@codecov

codecov Bot commented Aug 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.32%. Comparing base (114fd5d) to head (dbab806).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #1739   +/-   ##
========================================
  Coverage    97.32%   97.32%           
========================================
  Files          195      195           
  Lines        15739    15742    +3     
  Branches      3390     3393    +3     
========================================
+ Hits         15318    15321    +3     
  Misses         421      421           
Files with missing lines Coverage Δ
src/interpreter/plugin/AddressPlugin.ts 100.00% <ø> (ø)
src/interpreter/plugin/FunctionPlugin.ts 99.07% <100.00%> (+0.01%) ⬆️
🚀 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