feat: add setting to auto-expand diffs in chat messages#11316
feat: add setting to auto-expand diffs in chat messages#11316roomote[bot] wants to merge 1 commit intomainfrom
Conversation
Adds a new boolean setting `autoExpandDiffs` (default: false) that automatically expands diff views in "Roo wants to edit this file" chat messages. Diffs are still constrained to max-h-[300px] with scrollbar. Changes: - Add autoExpandDiffs to GlobalSettings schema (packages/types) - Add autoExpandDiffs to ExtensionState type (packages/types) - Wire setting through ClineProvider state to webview - Add auto-expand logic in ChatView for diff tool messages - Add UI toggle in Settings > UI section - Add English i18n translations - Update test fixtures Closes #10955
Reviewed. The settings plumbing, types, UI toggle, and i18n look correct. Found 2 issues in the auto-expand
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| useEffect(() => { | ||
| if (!autoExpandDiffs) { | ||
| return | ||
| } | ||
|
|
||
| const newExpansions: Record<number, boolean> = {} | ||
|
|
||
| for (const msg of modifiedMessages) { | ||
| // Skip messages already tracked in expandedRows | ||
| if (expandedRows[msg.ts] !== undefined) { | ||
| continue | ||
| } | ||
|
|
||
| // Check if this message contains a diff tool | ||
| if (msg.text) { | ||
| try { | ||
| const tool = JSON.parse(msg.text) | ||
| if (tool.tool && DIFF_TOOL_TYPES.has(tool.tool)) { | ||
| newExpansions[msg.ts] = true | ||
| } | ||
| } catch { | ||
| // Not valid JSON, skip | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (Object.keys(newExpansions).length > 0) { | ||
| setExpandedRows((prev) => ({ ...prev, ...newExpansions })) | ||
| } | ||
| }, [modifiedMessages, autoExpandDiffs, expandedRows, DIFF_TOOL_TYPES]) |
There was a problem hiding this comment.
Including expandedRows in the dependency array here means this effect re-runs on every expansion/collapse (user or auto), iterating all modifiedMessages and JSON.parse-ing each one. It also causes a double execution when it auto-expands: the setExpandedRows call updates expandedRows, which re-triggers the effect (the second run is a no-op but still wasteful). You can fix both problems by moving the logic into the functional updater of setExpandedRows and dropping expandedRows from deps:
| useEffect(() => { | |
| if (!autoExpandDiffs) { | |
| return | |
| } | |
| const newExpansions: Record<number, boolean> = {} | |
| for (const msg of modifiedMessages) { | |
| // Skip messages already tracked in expandedRows | |
| if (expandedRows[msg.ts] !== undefined) { | |
| continue | |
| } | |
| // Check if this message contains a diff tool | |
| if (msg.text) { | |
| try { | |
| const tool = JSON.parse(msg.text) | |
| if (tool.tool && DIFF_TOOL_TYPES.has(tool.tool)) { | |
| newExpansions[msg.ts] = true | |
| } | |
| } catch { | |
| // Not valid JSON, skip | |
| } | |
| } | |
| } | |
| if (Object.keys(newExpansions).length > 0) { | |
| setExpandedRows((prev) => ({ ...prev, ...newExpansions })) | |
| } | |
| }, [modifiedMessages, autoExpandDiffs, expandedRows, DIFF_TOOL_TYPES]) | |
| useEffect(() => { | |
| if (!autoExpandDiffs) { | |
| return | |
| } | |
| setExpandedRows((prev) => { | |
| const newExpansions: Record<number, boolean> = {} | |
| for (const msg of modifiedMessages) { | |
| if (prev[msg.ts] !== undefined) { | |
| continue | |
| } | |
| if (msg.text) { | |
| try { | |
| const tool = JSON.parse(msg.text) | |
| if (tool.tool && DIFF_TOOL_TYPES.has(tool.tool)) { | |
| newExpansions[msg.ts] = true | |
| } | |
| } catch { | |
| // Not valid JSON, skip | |
| } | |
| } | |
| } | |
| if (Object.keys(newExpansions).length > 0) { | |
| return { ...prev, ...newExpansions } | |
| } | |
| return prev | |
| }) | |
| }, [modifiedMessages, autoExpandDiffs, DIFF_TOOL_TYPES]) |
Fix it with Roo Code or mention @roomote and request a fix.
| useEffect(() => { | ||
| if (!autoExpandDiffs) { | ||
| return | ||
| } | ||
|
|
||
| const newExpansions: Record<number, boolean> = {} | ||
|
|
||
| for (const msg of modifiedMessages) { | ||
| // Skip messages already tracked in expandedRows | ||
| if (expandedRows[msg.ts] !== undefined) { | ||
| continue | ||
| } | ||
|
|
||
| // Check if this message contains a diff tool | ||
| if (msg.text) { | ||
| try { | ||
| const tool = JSON.parse(msg.text) | ||
| if (tool.tool && DIFF_TOOL_TYPES.has(tool.tool)) { | ||
| newExpansions[msg.ts] = true | ||
| } | ||
| } catch { | ||
| // Not valid JSON, skip | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (Object.keys(newExpansions).length > 0) { | ||
| setExpandedRows((prev) => ({ ...prev, ...newExpansions })) | ||
| } | ||
| }, [modifiedMessages, autoExpandDiffs, expandedRows, DIFF_TOOL_TYPES]) |
There was a problem hiding this comment.
When this effect auto-expands diffs via setExpandedRows, the existing effect at lines 509-529 will detect those transitions (from undefined to true) and set stickyFollowRef.current = false, disabling auto-scroll. It cannot distinguish between user-initiated and programmatic expansions. During an active task with autoExpandDiffs enabled, each new diff message gets auto-expanded, triggers that detection, and disables auto-scroll -- the user has to manually scroll to see subsequent messages, which undermines the purpose of this feature. Consider guarding against this by setting a ref flag (e.g. isAutoExpandingRef.current = true) before calling setExpandedRows here, and checking it in the expansion-tracking effect to skip the sticky-follow disable.
Fix it with Roo Code or mention @roomote and request a fix.
1 participant
Related GitHub Issue
Closes: #10955
Description
This PR attempts to address Issue #10955. It adds a new boolean setting
autoExpandDiffs(default:false) that automatically expands diff views in "Roo wants to edit this file" chat messages.Key implementation details:
reasoningBlockCollapsedfor the full settings flowuseEffectin ChatView detects diff tool messages (editedExistingFile,appliedDiff,insertContent,searchAndReplace,newFileCreated) and auto-expands themmax-h-[300px](300px) withoverflow-y: auto, so even auto-expanded diffs will not be too verbose -- they get a scrollbar for anything taller than 300pxfalseto preserve existing behavior (collapsed diffs)Files changed:
packages/types/src/global-settings.ts-- AddautoExpandDiffsto schemapackages/types/src/vscode-extension-host.ts-- Add to ExtensionState typesrc/core/webview/ClineProvider.ts-- Wire setting through statewebview-ui/src/context/ExtensionStateContext.tsx-- Default value and contextwebview-ui/src/components/chat/ChatView.tsx-- Auto-expand logicwebview-ui/src/components/settings/UISettings.tsx-- UI togglewebview-ui/src/components/settings/SettingsView.tsx-- Wire propwebview-ui/src/i18n/locales/en/settings.json-- English translationsFeedback and guidance are welcome.
Test Procedure
Automated tests:
UISettings.spec.tsx-- 4/4 passing (updated with new prop)SettingsView.change-detection.spec.tsx-- 3/3 passing (updated fixtures)SettingsView.unsaved-changes.spec.tsx-- 3/3 passing (updated fixtures)ExtensionStateContext.spec.tsx-- 9/9 passingPre-Submission Checklist
Documentation Updates
Additional Notes
This addresses the user's concern about verbosity: the
CodeAccordiancomponent appliesmax-h-[300px]on the expanded content container, so even auto-expanded diffs are height-constrained with a scrollbar.Important
Adds
autoExpandDiffssetting to auto-expand diffs in chat messages, with UI and tests updated accordingly.autoExpandDiffssetting to auto-expand diffs in chat messages, defaulting tofalse.ChatView.tsxusesuseEffectto auto-expand diffs for specific message types when setting is enabled.UISettings.tsxandSettingsView.tsxupdated to includeautoExpandDiffstoggle.global-settings.tsschema updated to includeautoExpandDiffs.UISettings.spec.tsx,SettingsView.change-detection.spec.tsx, andSettingsView.unsaved-changes.spec.tsxto cover new setting.settings.jsonfor new setting.This description was created by
for d1bddd3. You can customize this summary. It will automatically update as commits are pushed.