Skip to content

[Vulcan] Document workbook-wide filter wrapper invalidation - #339

Open
iliaal wants to merge 1 commit into
masterfrom
vulcan/removefilter-wrapper-contract-20261025
Open

iliaal wants to merge 1 commit into
masterfrom
vulcan/removefilter-wrapper-contract-20261025

Conversation

@iliaal

@iliaal iliaal commented Oct 25, 2026 •

Copy link
Copy Markdown
Owner

Summary

Document the existing workbook-wide wrapper invalidation performed by ExcelSheet::removeFilter() and ExcelTable::removeFilter(). Even a no-op removal invalidates all existing autofilter/filter-column wrappers in that workbook; owners and filters in other workbooks survive. Callers must re-fetch surviving filters. No runtime behavior changes.

Add regression coverage for both sheet/table directions, no-op removals, surviving owners, re-fetched filter ranges, and workbook isolation. Test 260 runs on the supported LibXL floor; table coverage in 261 requires LibXL 5.2. Numbers avoid sibling PR #338's test 259.

Validation

  • Strict native PHP 8.4.26 NTS / LibXL 5.2.0.1 build with repository developer warnings: no diagnostics.
  • Final PHPT suite: 256 passed, 7 skipped, 0 failed/warned. Skips are license/platform-specific, not passes.
  • Both new tests fail when both autofilter-generation bumps are omitted, and pass against unchanged runtime.
  • Changed PHP documentation syntax checks and git diff --check pass; independent native/source review passed.
  • Source/sibling sweep: the only autofilter-generation bump callers are these two removal APIs. Conditional-format removal uses a separate generation and is outside this focused change. No contradictory filter lifetime prose found.
  • Actual merge-tree checks clean with current master and sibling PRs [Vulcan] Clarify nullable properties and indexed range warnings #337/[Vulcan] Document moveSheet wrapper invalidation #338; remote tree/modes match the locally tested tree.

Limitations: native build reused official headers/library, not a fresh phpize/configure; no local Windows/ZTS/sanitizer or performance claim. GitHub CI for exact head 89ef1c72bf19691daf16f96a3d88cd06fc0af13a completed successfully: Tests and Windows Build, 36 jobs passed, 1 release-attachment job intentionally skipped (not a pass). All 36 job logs verified against the exact-head merge commit; test summaries show zero failures/warnings. Regression 260 executes on LibXL 4.6/5.1/5.2 lanes; 261 is version-gated to 5.2; both execute in PHP 8.4 ZTS/ASAN lanes.

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.

1 participant