Skip to content

SCM - refactor history item hover for the graph #278778

Merged
lszomoru merged 5 commits intomainfrom
lszomoru/scm-hover-refactor
Nov 21, 2025
Merged

SCM - refactor history item hover for the graph #278778
lszomoru merged 5 commits intomainfrom
lszomoru/scm-hover-refactor

Conversation

@lszomoru
Copy link
Member

No description provided.

Copilot AI review requested due to automatic review settings November 21, 2025 13:57
@lszomoru lszomoru enabled auto-merge (squash) November 21, 2025 13:57
@lszomoru lszomoru self-assigned this Nov 21, 2025
@vs-code-engineering vs-code-engineering bot added this to the November 2025 milestone Nov 21, 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 the SCM history item hover rendering to support arrays of MarkdownString objects for the graph view. The main changes enable VS Code to inject reference badges with appropriate colors into history item hovers, since extensions don't have access to the graph coloring information.

  • Extends the tooltip API to support arrays of MarkdownString objects instead of just strings or single MarkdownStrings
  • Moves hover rendering logic from the git extension to VS Code's SCM view layer
  • Creates a new shared hover.ts module in the git extension for reusable hover generation functions
  • Updates CSS selectors to support the new multi-markdown structure

Reviewed changes

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

Show a summary per file
File Description
src/vscode-dts/vscode.proposed.scmHistoryProvider.d.ts Updates tooltip API to support MarkdownString | Array<MarkdownString> instead of string | MarkdownString
src/vs/workbench/contrib/scm/common/history.ts Updates internal tooltip type to match the new API definition
src/vs/workbench/contrib/scm/browser/scmHistoryViewPane.ts Adds _renderHoverContent method to handle tooltip arrays and inject reference badges with colors
src/vs/workbench/contrib/scm/browser/media/scm.css Updates CSS selectors to target the new .history-item-hover-container and .rendered-markdown structure
src/vs/workbench/api/common/extHostSCM.ts Converts extension tooltip data using MarkdownString.fromMany for arrays
src/vs/workbench/api/common/extHost.protocol.ts Updates the DTO type definition to match the new API
extensions/git/src/hover.ts New file containing shared hover generation functions extracted from historyProvider.ts
extensions/git/src/historyProvider.ts Removes hover rendering logic and references/backgroundColor properties from history item refs
extensions/git/src/timelineProvider.ts Updates to use the new hover functions from hover.ts
extensions/git/src/blame.ts Updates to use the new hover functions from hover.ts

}

markdownString.appendMarkdown(`\n\n---\n\n`);
historyItem.tooltip.splice(historyItem.tooltip.length - 1, 0, markdownString);
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

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

Direct mutation of the tooltip array should be avoided as it modifies the original data structure from the extension. This can lead to unexpected behavior if the tooltip is rendered multiple times or cached by the extension. Consider creating a new array instead:

const tooltipWithReferences = [...historyItem.tooltip];
tooltipWithReferences.splice(tooltipWithReferences.length - 1, 0, markdownString);

Then iterate over tooltipWithReferences in the loop below instead of historyItem.tooltip.

Copilot uses AI. Check for mistakes.
bpasero
bpasero previously approved these changes Nov 21, 2025
@lszomoru lszomoru merged commit a735882 into main Nov 21, 2025
28 checks passed
@lszomoru lszomoru deleted the lszomoru/scm-hover-refactor branch November 21, 2025 15:48
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Jan 5, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants