Skip to content

Conversation

@Abe27342
Copy link
Contributor

Description

Makes IMergeTreeTextHelper take in a perspective rather than refSeq + clientId. To make this usable with the local perspective in SharedString, I've also added a localPerspective field to the collab window.

Copilot AI review requested due to automatic review settings March 24, 2025 20:14
@Abe27342 Abe27342 requested a review from a team as a code owner March 24, 2025 20:14
@github-actions github-actions bot added area: dds Issues related to distributed data structures area: dds: sharedstring base: main PRs targeted against main branch labels Mar 24, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors MergeTreeTextHelper and related components to use a unified Perspective object in place of the separate refSeq and clientId parameters. The changes update API signatures, type exports, and internal calls across the merge-tree and sharedString packages to support a local perspective for collaboration.

  • Changed API for MergeTreeTextHelper#getText to accept a Perspective.
  • Updated CollaborationWindow to expose a localPerspective.
  • Modified tests and client/server code to use the new perspective-based approach.

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.

Show a summary per file
File Description
packages/dds/merge-tree/src/mergeTreeNodes.ts Added LocalDefaultPerspective import and initialized localPerspective in CollaborationWindow.
packages/dds/merge-tree/src/MergeTreeTextHelper.ts Replaced separate refSeq/clientId parameters with a single Perspective parameter.
packages/dds/merge-tree/src/test/beastTest.spec.ts Updated getText() calls to use mergeTree.localPerspective.
packages/dds/merge-tree/src/index.ts Adjusted type exports to reflect changes in text helper interfaces.
packages/dds/merge-tree/src/test/testServer.ts Updated perspective creation in checkTextMatchRelative calls.
packages/dds/merge-tree/src/mergeTree.ts Changed localPerspective to a getter that retrieves data from the collabWindow.
packages/dds/merge-tree/src/textSegment.ts Removed deprecated interface for IMergeTreeTextHelper.
packages/dds/merge-tree/src/test/mergeTree.insertingWalk.spec.ts Updated getText() calls in test assertions.
packages/dds/merge-tree/src/test/testClient.ts Updated getText() and relText() usage with the new perspective approach.
packages/dds/sequence/src/sharedString.ts Updated getText() and getTextWithPlaceholders() calls to use the localPerspective.
packages/dds/merge-tree/src/client.ts Adjusted type imports to be consistent with the refactor.
packages/dds/merge-tree/src/perspective.ts Added @internal documentation to clarify the perspective interface.
Comments suppressed due to low confidence (1)

packages/dds/merge-tree/src/test/testClient.ts:318

  • [nitpick] The use of a newly constructed PriorPerspective in relText may be inconsistent with other parts of the code that rely on mergeTree.localPerspective. Consider reviewing this call to ensure it aligns with the intended behavior for retrieving client text representations.
)} refSeq: ${refSeq}: ${this.textHelper.getText(new PriorPerspective(refSeq, clientId))}`;

@Abe27342 Abe27342 merged commit c78b48f into microsoft:main Mar 28, 2025
33 checks passed
@Abe27342 Abe27342 deleted the perspective-on-text-helper branch March 28, 2025 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: dds: sharedstring area: dds Issues related to distributed data structures base: main PRs targeted against main branch

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants