Skip to content

Update NSDiffableDataSource to be more compliant with the macOS implementation - #912

Open
gcasa wants to merge 6 commits into
masterfrom
diffable_datasource_fixes
Open

Update NSDiffableDataSource to be more compliant with the macOS implementation#912
gcasa wants to merge 6 commits into
masterfrom
diffable_datasource_fixes

Conversation

@gcasa

@gcasa gcasa commented Aug 14, 2026

Copy link
Copy Markdown
Member

There were some issues with the initial implementation. These changes attempt to address some of those issues.

AI Disclosure: AI was used in this change to help create tests, documentation, and detect bugs.

@gcasa
gcasa marked this pull request as ready for review August 15, 2026 11:13
@gcasa
gcasa requested a review from fredkiefer as a code owner August 15, 2026 11:13
@fredkiefer
fredkiefer removed their request for review September 6, 2026 15:38
@gcasa
gcasa requested review from fredkiefer and rfm and a lite review from Copilot September 7, 2026 19:46

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

There are API/behavioral issues that should be corrected before merge (notably NSTableView’s nil-identifier registerNib:forIdentifier: handling, plus avoidable update-path reload overhead in the animated snapshot application flow).

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR refines GNUstep’s NSDiffableDataSource / snapshot behavior and expands API surface + tests to better align with expected macOS semantics, including item/section mutation operations, reload tracking, and additional provider hooks.

Changes:

  • Extend NSDiffableDataSourceSnapshot with item move APIs, reload tracking, and stricter duplicate identifier validation.
  • Add/extend diffable data source APIs (completion handler, supplementary/row/section-header providers, table row/section mapping helpers) and expand GUI tests accordingly.
  • Harden NSTableView view-registration paths to avoid registering prototype views with nil identifiers.
File summaries
File Description
Tests/gui/NSTableView/TestInfo Adds/records NSTableView test suite metadata.
Tests/gui/NSTableView/columns.m Adds coverage for registering prototype views with nil identifiers.
Tests/gui/NSDiffableDataSource/NSDiffableDataSource_snapshot.m Adds tests for item moves and duplicate item identifier rejection.
Tests/gui/NSDiffableDataSource/NSDiffableDataSource_collectionView.m Strengthens snapshot/data-source integration assertions; adds table diffable row-mapping coverage.
Source/NSTableView.m Skips nil identifiers in prototype registration; changes registerNib:forIdentifier: nil handling.
Source/NSDiffableDataSource.m Adds snapshot reload tracking + item moves; adds completion handler APIs; adds supplementary/row/header providers; improves table diff application behavior.
Headers/AppKit/NSDiffableDataSource.h Exposes new snapshot ops and provider/completion APIs in public headers.
Review details
  • Files reviewed: 7/7 changed files
  • Comments generated: 2
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread Source/NSDiffableDataSource.m Outdated
Comment thread Source/NSTableView.m
gcasa and others added 2 commits September 7, 2026 15:58
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants