Skip to content

Conversation

@alexr00
Copy link
Member

@alexr00 alexr00 commented Nov 10, 2025

Fixes #8050

@alexr00 alexr00 self-assigned this Nov 10, 2025
@alexr00 alexr00 requested a review from Copilot November 10, 2025 15:48
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 introduces reference counting for commenting range providers and fixes the comment controller ID generation to include remote names, addressing potential conflicts when multiple remotes point to the same repository.

Key changes:

  • Implements reference counting for CommentingRangeProvider2 instances to prevent premature disposal
  • Updates comment controller ID to include remoteName for better uniqueness
  • Moves await this.ensure() before the early return check in ensureCommentsController()

Reviewed Changes

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

File Description
src/view/pullRequestCommentControllerRegistry.ts Adds PRCommentingRangeProviderInfo interface with reference counting, updates provider registration/disposal logic to match the existing pattern used for comment handlers
src/github/githubRepository.ts Adds remoteName to comment controller ID and reorders initialization to ensure proper setup before early return

@alexr00 alexr00 marked this pull request as ready for review November 10, 2025 15:56
@alexr00 alexr00 enabled auto-merge (squash) November 10, 2025 15:57
@vs-code-engineering vs-code-engineering bot added this to the November 2025 milestone Nov 10, 2025
Comment on lines +23 to +27
interface PRCommentingRangeProviderInfo {
provider: vscode.CommentingRangeProvider2;
refCount: number;
dispose: () => void;
}
Copy link
Member

Choose a reason for hiding this comment

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

extends IDisposable?

@alexr00 alexr00 merged commit a745d1f into main Nov 10, 2025
12 checks passed
@alexr00 alexr00 deleted the alexr00/issue8050 branch November 10, 2025 16:02
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.

Comments don't show when PR is on non-default repo

3 participants