-
Notifications
You must be signed in to change notification settings - Fork 673
Show commit count as decoration on Commits tree node #7959
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: alexr00 <[email protected]>
|
Cool! I could try it out if there was a way for me to build or pull down a pre-built VS Code to test. Is that a thing that exists? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@andyshinn, not really. But, once the PR is merged then the next day you'll be able to try it out in the pre-release version of the GitHub Pull Request extension.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot, don't use the description of the tree node. Instead use a file decoration.
Co-authored-by: alexr00 <[email protected]>
Done in 861832d. Changed to use a file decoration with badge showing the commit count instead of using the tree node's description property. The implementation:
The decoration will appear as a badge next to the "Commits" label in the tree view, similar to how other decorations work in VS Code (e.g., file status badges). |
| export interface CommitsNodeUriParams { | ||
| prNumber: number; | ||
| commitsCount: number; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot Should also include the owner and repo in the params
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in eaaa39d. Added owner and repo to CommitsNodeUriParams and updated the createCommitsNodeUri function to accept these parameters.
src/common/uri.ts
Outdated
| commitsCount | ||
| }; | ||
|
|
||
| return vscode.Uri.parse(`commitsnode:${prNumber}`).with({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot use Schemes.CommitsNode instead of using the string directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in eaaa39d. Changed to use Schemes.CommitsNode instead of the string literal 'commitsnode'.
|
|
||
| registerPullRequestPropertyChangedListeners(_folderManager: FolderRepositoryManager, _model: PullRequestModel): vscode.Disposable { | ||
| // No need to listen for changes since commit count doesn't change dynamically | ||
| return { dispose: () => { } }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot this can change, when for example the pr timeline changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in eaaa39d. Updated to listen for onDidChange events with timeline: true and fire decoration updates when the PR timeline changes, which includes new commits being added.
…for timeline changes Co-authored-by: alexr00 <[email protected]>
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.