Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the diff statistics display in the agent sessions viewer by moving it from the renderer into a dedicated action view item. The change improves code organization by separating UI concerns and making the diff statistics interactive.
Key changes:
- Extracted diff statistics rendering logic from the renderer into a new
AgentSessionShowDiffActionandAgentSessionDiffActionViewItem - Replaced inline diff elements with an ActionBar-based toolbar
- Moved associated CSS styles from
agentsessionsviewer.cssto a newagentsessionsactions.cssfile
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| agentsessionsviewer.css | Removed inline diff styling and related color selectors, added padding for status element |
| agentsessionsactions.css | New file containing diff container styles and color definitions for files/additions/deletions |
| agentSessionsViewer.ts | Replaced inline diff elements with ActionBar toolbar integration and added action view item provider |
| agentSessionsActions.ts | New file implementing AgentSessionShowDiffAction and AgentSessionDiffActionViewItem for interactive diff statistics |
|
|
||
| const session = this.action.getSession(); | ||
|
|
||
| this.commandService.executeCommand(`agentSession.${session.provider.chatSessionType}.openChanges`, this.action.getSession().resource); |
There was a problem hiding this comment.
The session is retrieved twice unnecessarily. The session variable is already defined on line 81 but this.action.getSession() is called again to get the resource. Use session.resource instead.
| this.commandService.executeCommand(`agentSession.${session.provider.chatSessionType}.openChanges`, this.action.getSession().resource); | |
| this.commandService.executeCommand(`agentSession.${session.provider.chatSessionType}.openChanges`, session.resource); |
| elements.filesSpan.textContent = diff.files > 0 ? `${diff.files}` : ''; | ||
| elements.addedSpan.textContent = diff.insertions > 0 ? `+${diff.insertions}` : ''; | ||
| elements.removedSpan.textContent = diff.deletions > 0 ? `-${diff.deletions}` : ''; |
There was a problem hiding this comment.
This logic duplicates the condition check from line 138 in agentSessionsViewer.ts. The action is only created when diff.files > 0 || diff.insertions > 0 || diff.deletions > 0, so these checks in the render method may be redundant. Consider simplifying to always display the values when the action exists, or document why the double-checking is necessary.
| elements.filesSpan.textContent = diff.files > 0 ? `${diff.files}` : ''; | |
| elements.addedSpan.textContent = diff.insertions > 0 ? `+${diff.insertions}` : ''; | |
| elements.removedSpan.textContent = diff.deletions > 0 ? `-${diff.deletions}` : ''; | |
| elements.filesSpan.textContent = `${diff.files}`; | |
| elements.addedSpan.textContent = `+${diff.insertions}`; | |
| elements.removedSpan.textContent = `-${diff.deletions}`; |
No description provided.