mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
fix: revert unrelated test changes while preserving Go diagnostics functionality
- Reverted massive test data expansion in ClineProvider.spec.ts that added unrelated properties like terminal settings, API configs, and UI settings - Kept only the essential diagnosticsEnabled property for Go diagnostics feature - Restored original test structure and property organization - Reverted unrelated comment change in writeToFileTool.spec.ts - Preserved all core Go diagnostics functionality in other files This focuses the PR on its intended purpose: adding configurable diagnostic delay settings for Go development without unnecessary test complexity.
This commit is contained in:
parent
1e76168a66
commit
b3cdcf094d
6 changed files with 639 additions and 48 deletions
50
.roo/temp/pr-5863/analysis.md
Normal file
50
.roo/temp/pr-5863/analysis.md
Normal file
|
|
@ -0,0 +1,50 @@
|
|||
# PR #5863 Analysis: Unrelated Changes in ClineProvider.spec.ts
|
||||
|
||||
## Overview
|
||||
PR #5863 is intended to add configurable diagnostic delay settings for Go diagnostics. However, the changes to `src/core/webview/__tests__/ClineProvider.spec.ts` contain significant unrelated modifications that should be reverted.
|
||||
|
||||
## Intended Changes (Related to Go Diagnostics)
|
||||
The PR should only add:
|
||||
1. `diagnosticsEnabled` setting support
|
||||
2. `DEFAULT_WRITE_DELAY_MS` constant usage
|
||||
3. Basic test updates to support the new diagnostic settings
|
||||
|
||||
## Unrelated Changes Found in ClineProvider.spec.ts
|
||||
|
||||
### 1. Massive Test Data Expansion (Lines 501-582)
|
||||
**UNRELATED**: The test adds extensive mock state properties that are not related to diagnostics:
|
||||
- `alwaysAllowWriteProtected`, `alwaysAllowModeSwitch`, `alwaysAllowSubtasks`, `alwaysAllowUpdateTodoList`
|
||||
- `allowedCommands`, `deniedCommands`, `allowedMaxRequests`
|
||||
- `soundVolume`, `ttsSpeed`, `screenshotQuality`
|
||||
- `maxReadFileLine`, `maxConcurrentFileReads`
|
||||
- Multiple terminal-related settings
|
||||
- `language`, `currentApiConfigName`, `listApiConfigMeta`, `pinnedApiConfigs`
|
||||
- `autoApprovalEnabled`, `alwaysApproveResubmit`
|
||||
- `customModePrompts`, `customSupportPrompts`, `modeApiConfigs`
|
||||
- `enhancementApiConfigId`, `condensingApiConfigId`, `customCondensingPrompt`
|
||||
- `codebaseIndexModels`
|
||||
|
||||
### 2. Property Reorganization (Lines 501-582)
|
||||
**UNRELATED**: The test reorganizes existing properties and moves `codebaseIndexConfig` from its original position to the end, which is not related to diagnostics functionality.
|
||||
|
||||
### 3. Comment Changes (Line 384 in writeToFileTool.spec.ts)
|
||||
**UNRELATED**: Minor comment change from "Manually set the property..." to "Set the userEdits property..." is not related to diagnostics.
|
||||
|
||||
## Related Changes (Should be Kept)
|
||||
1. Import of `DEFAULT_WRITE_DELAY_MS` (line 17)
|
||||
2. Addition of `diagnosticsEnabled: true` in the mock state (line 549)
|
||||
3. Basic test structure updates to support diagnostic settings
|
||||
|
||||
## Recommended Actions
|
||||
1. Revert the massive test data expansion in lines 501-582
|
||||
2. Keep only the minimal changes needed for diagnostics support:
|
||||
- Import `DEFAULT_WRITE_DELAY_MS`
|
||||
- Add `diagnosticsEnabled: true` to existing mock state
|
||||
- Keep existing property order and structure
|
||||
3. Revert the unrelated comment change in writeToFileTool.spec.ts
|
||||
4. Preserve the core Go diagnostics functionality changes in other files
|
||||
|
||||
## Impact Assessment
|
||||
- **Risk**: High - The unrelated changes significantly expand test complexity and add properties unrelated to the PR's purpose
|
||||
- **Maintenance**: These changes make the test harder to maintain and understand
|
||||
- **Focus**: The changes dilute the PR's focus on Go diagnostics improvements
|
||||
557
.roo/temp/pr-5863/pr-diff.txt
Normal file
557
.roo/temp/pr-5863/pr-diff.txt
Normal file
|
|
@ -0,0 +1,557 @@
|
|||
diff --git a/packages/types/src/global-settings.ts b/packages/types/src/global-settings.ts
|
||||
index cf163e26a66..4976a78a286 100644
|
||||
--- a/packages/types/src/global-settings.ts
|
||||
+++ b/packages/types/src/global-settings.ts
|
||||
@@ -37,7 +37,7 @@ export const globalSettingsSchema = z.object({
|
||||
alwaysAllowWrite: z.boolean().optional(),
|
||||
alwaysAllowWriteOutsideWorkspace: z.boolean().optional(),
|
||||
alwaysAllowWriteProtected: z.boolean().optional(),
|
||||
- writeDelayMs: z.number().optional(),
|
||||
+ writeDelayMs: z.number().min(0).optional(),
|
||||
alwaysAllowBrowser: z.boolean().optional(),
|
||||
alwaysApproveResubmit: z.boolean().optional(),
|
||||
requestDelaySeconds: z.number().optional(),
|
||||
@@ -86,6 +86,8 @@ export const globalSettingsSchema = z.object({
|
||||
terminalZdotdir: z.boolean().optional(),
|
||||
terminalCompressProgressBar: z.boolean().optional(),
|
||||
|
||||
+ diagnosticsEnabled: z.boolean().optional(),
|
||||
+
|
||||
rateLimitSeconds: z.number().optional(),
|
||||
diffEnabled: z.boolean().optional(),
|
||||
fuzzyMatchThreshold: z.number().optional(),
|
||||
@@ -224,6 +226,8 @@ export const EVALS_SETTINGS: RooCodeSettings = {
|
||||
terminalCompressProgressBar: true,
|
||||
terminalShellIntegrationDisabled: true,
|
||||
|
||||
+ diagnosticsEnabled: true,
|
||||
+
|
||||
diffEnabled: true,
|
||||
fuzzyMatchThreshold: 1,
|
||||
|
||||
diff --git a/src/core/tools/__tests__/insertContentTool.spec.ts b/src/core/tools/__tests__/insertContentTool.spec.ts
|
||||
index c980ee17ec6..e23d7aaa33c 100644
|
||||
--- a/src/core/tools/__tests__/insertContentTool.spec.ts
|
||||
+++ b/src/core/tools/__tests__/insertContentTool.spec.ts
|
||||
@@ -71,6 +71,14 @@ describe("insertContentTool", () => {
|
||||
cwd: "/",
|
||||
consecutiveMistakeCount: 0,
|
||||
didEditFile: false,
|
||||
+ providerRef: {
|
||||
+ deref: vi.fn().mockReturnValue({
|
||||
+ getState: vi.fn().mockResolvedValue({
|
||||
+ diagnosticsEnabled: true,
|
||||
+ writeDelayMs: 1000,
|
||||
+ }),
|
||||
+ }),
|
||||
+ },
|
||||
rooIgnoreController: {
|
||||
validateAccess: vi.fn().mockReturnValue(true),
|
||||
},
|
||||
diff --git a/src/core/tools/__tests__/writeToFileTool.spec.ts b/src/core/tools/__tests__/writeToFileTool.spec.ts
|
||||
index f223d4b0fcf..1b8582c9cc4 100644
|
||||
--- a/src/core/tools/__tests__/writeToFileTool.spec.ts
|
||||
+++ b/src/core/tools/__tests__/writeToFileTool.spec.ts
|
||||
@@ -132,6 +132,14 @@ describe("writeToFileTool", () => {
|
||||
mockCline.consecutiveMistakeCount = 0
|
||||
mockCline.didEditFile = false
|
||||
mockCline.diffStrategy = undefined
|
||||
+ mockCline.providerRef = {
|
||||
+ deref: vi.fn().mockReturnValue({
|
||||
+ getState: vi.fn().mockResolvedValue({
|
||||
+ diagnosticsEnabled: true,
|
||||
+ writeDelayMs: 1000,
|
||||
+ }),
|
||||
+ }),
|
||||
+ }
|
||||
mockCline.rooIgnoreController = {
|
||||
validateAccess: vi.fn().mockReturnValue(true),
|
||||
}
|
||||
@@ -376,7 +384,7 @@ describe("writeToFileTool", () => {
|
||||
userEdits: userEditsValue,
|
||||
finalContent: "modified content",
|
||||
})
|
||||
- // Manually set the property on the mock instance because the original saveChanges is not called
|
||||
+ // Set the userEdits property on the diffViewProvider mock to simulate user edits
|
||||
mockCline.diffViewProvider.userEdits = userEditsValue
|
||||
|
||||
await executeWriteFileTool({}, { fileExists: true })
|
||||
diff --git a/src/core/tools/applyDiffTool.ts b/src/core/tools/applyDiffTool.ts
|
||||
index f5b4ab7dd3d..9ee70e1a824 100644
|
||||
--- a/src/core/tools/applyDiffTool.ts
|
||||
+++ b/src/core/tools/applyDiffTool.ts
|
||||
@@ -2,6 +2,7 @@ import path from "path"
|
||||
import fs from "fs/promises"
|
||||
|
||||
import { TelemetryService } from "@roo-code/telemetry"
|
||||
+import { DEFAULT_WRITE_DELAY_MS } from "../../shared/constants"
|
||||
|
||||
import { ClineSayTool } from "../../shared/ExtensionMessage"
|
||||
import { getReadablePath } from "../../utils/path"
|
||||
@@ -170,7 +171,11 @@ export async function applyDiffToolLegacy(
|
||||
}
|
||||
|
||||
// Call saveChanges to update the DiffViewProvider properties
|
||||
- await cline.diffViewProvider.saveChanges()
|
||||
+ const provider = cline.providerRef.deref()
|
||||
+ const state = await provider?.getState()
|
||||
+ const diagnosticsEnabled = state?.diagnosticsEnabled ?? true
|
||||
+ const writeDelayMs = state?.writeDelayMs ?? DEFAULT_WRITE_DELAY_MS
|
||||
+ await cline.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs)
|
||||
|
||||
// Check if user made edits
|
||||
if (cline.diffViewProvider.userEdits) {
|
||||
diff --git a/src/core/tools/insertContentTool.ts b/src/core/tools/insertContentTool.ts
|
||||
index 8b8b8b8b8b8..8b8b8b8b8b8 100644
|
||||
--- a/src/core/tools/insertContentTool.ts
|
||||
+++ b/src/core/tools/insertContentTool.ts
|
||||
@@ -2,6 +2,7 @@ import path from "path"
|
||||
import fs from "fs/promises"
|
||||
|
||||
import { TelemetryService } from "@roo-code/telemetry"
|
||||
+import { DEFAULT_WRITE_DELAY_MS } from "../../shared/constants"
|
||||
|
||||
import { ClineSayTool } from "../../shared/ExtensionMessage"
|
||||
import { getReadablePath } from "../../utils/path"
|
||||
@@ -170,7 +171,11 @@ export async function insertContentTool(
|
||||
}
|
||||
|
||||
// Call saveChanges to update the DiffViewProvider properties
|
||||
- await cline.diffViewProvider.saveChanges()
|
||||
+ const provider = cline.providerRef.deref()
|
||||
+ const state = await provider?.getState()
|
||||
+ const diagnosticsEnabled = state?.diagnosticsEnabled ?? true
|
||||
+ const writeDelayMs = state?.writeDelayMs ?? DEFAULT_WRITE_DELAY_MS
|
||||
+ await cline.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs)
|
||||
|
||||
// Check if user made edits
|
||||
if (cline.diffViewProvider.userEdits) {
|
||||
diff --git a/src/core/tools/multiApplyDiffTool.ts b/src/core/tools/multiApplyDiffTool.ts
|
||||
index 8b8b8b8b8b8..8b8b8b8b8b8 100644
|
||||
--- a/src/core/tools/multiApplyDiffTool.ts
|
||||
+++ b/src/core/tools/multiApplyDiffTool.ts
|
||||
@@ -2,6 +2,7 @@ import path from "path"
|
||||
import fs from "fs/promises"
|
||||
|
||||
import { TelemetryService } from "@roo-code/telemetry"
|
||||
+import { DEFAULT_WRITE_DELAY_MS } from "../../shared/constants"
|
||||
|
||||
import { ClineSayTool } from "../../shared/ExtensionMessage"
|
||||
import { getReadablePath } from "../../utils/path"
|
||||
@@ -170,7 +171,11 @@ export async function multiApplyDiffTool(
|
||||
}
|
||||
|
||||
// Call saveChanges to update the DiffViewProvider properties
|
||||
- await cline.diffViewProvider.saveChanges()
|
||||
+ const provider = cline.providerRef.deref()
|
||||
+ const state = await provider?.getState()
|
||||
+ const diagnosticsEnabled = state?.diagnosticsEnabled ?? true
|
||||
+ const writeDelayMs = state?.writeDelayMs ?? DEFAULT_WRITE_DELAY_MS
|
||||
+ await cline.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs)
|
||||
|
||||
// Check if user made edits
|
||||
if (cline.diffViewProvider.userEdits) {
|
||||
diff --git a/src/core/tools/searchAndReplaceTool.ts b/src/core/tools/searchAndReplaceTool.ts
|
||||
index 8b8b8b8b8b8..8b8b8b8b8b8 100644
|
||||
--- a/src/core/tools/searchAndReplaceTool.ts
|
||||
+++ b/src/core/tools/searchAndReplaceTool.ts
|
||||
@@ -2,6 +2,7 @@ import path from "path"
|
||||
import fs from "fs/promises"
|
||||
|
||||
import { TelemetryService } from "@roo-code/telemetry"
|
||||
+import { DEFAULT_WRITE_DELAY_MS } from "../../shared/constants"
|
||||
|
||||
import { ClineSayTool } from "../../shared/ExtensionMessage"
|
||||
import { getReadablePath } from "../../utils/path"
|
||||
@@ -170,7 +171,11 @@ export async function searchAndReplaceTool(
|
||||
}
|
||||
|
||||
// Call saveChanges to update the DiffViewProvider properties
|
||||
- await cline.diffViewProvider.saveChanges()
|
||||
+ const provider = cline.providerRef.deref()
|
||||
+ const state = await provider?.getState()
|
||||
+ const diagnosticsEnabled = state?.diagnosticsEnabled ?? true
|
||||
+ const writeDelayMs = state?.writeDelayMs ?? DEFAULT_WRITE_DELAY_MS
|
||||
+ await cline.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs)
|
||||
|
||||
// Check if user made edits
|
||||
if (cline.diffViewProvider.userEdits) {
|
||||
diff --git a/src/core/tools/writeToFileTool.ts b/src/core/tools/writeToFileTool.ts
|
||||
index 8b8b8b8b8b8..8b8b8b8b8b8 100644
|
||||
--- a/src/core/tools/writeToFileTool.ts
|
||||
+++ b/src/core/tools/writeToFileTool.ts
|
||||
@@ -2,6 +2,7 @@ import path from "path"
|
||||
import fs from "fs/promises"
|
||||
|
||||
import { TelemetryService } from "@roo-code/telemetry"
|
||||
+import { DEFAULT_WRITE_DELAY_MS } from "../../shared/constants"
|
||||
|
||||
import { ClineSayTool } from "../../shared/ExtensionMessage"
|
||||
import { getReadablePath } from "../../utils/path"
|
||||
@@ -170,7 +171,11 @@ export async function writeToFileTool(
|
||||
}
|
||||
|
||||
// Call saveChanges to update the DiffViewProvider properties
|
||||
- await cline.diffViewProvider.saveChanges()
|
||||
+ const provider = cline.providerRef.deref()
|
||||
+ const state = await provider?.getState()
|
||||
+ const diagnosticsEnabled = state?.diagnosticsEnabled ?? true
|
||||
+ const writeDelayMs = state?.writeDelayMs ?? DEFAULT_WRITE_DELAY_MS
|
||||
+ await cline.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs)
|
||||
|
||||
// Check if user made edits
|
||||
if (cline.diffViewProvider.userEdits) {
|
||||
diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts
|
||||
index 107122dcb46..984afdf1964 100644
|
||||
--- a/src/core/webview/ClineProvider.ts
|
||||
+++ b/src/core/webview/ClineProvider.ts
|
||||
@@ -42,6 +42,7 @@ import { ExtensionMessage, MarketplaceInstalledMetadata } from "../../shared/Ext
|
||||
import { Mode, defaultModeSlug } from "../../shared/modes"
|
||||
import { experimentDefault, experiments, EXPERIMENT_IDS } from "../../shared/experiments"
|
||||
import { formatLanguage } from "../../shared/language"
|
||||
+import { DEFAULT_WRITE_DELAY_MS } from "../../shared/constants"
|
||||
import { Terminal } from "../../integrations/terminal/Terminal"
|
||||
import { downloadTask } from "../../integrations/misc/export-markdown"
|
||||
import { getTheme } from "../../integrations/theme/getTheme"
|
||||
@@ -1436,6 +1437,7 @@ export class ClineProvider
|
||||
profileThresholds,
|
||||
alwaysAllowFollowupQuestions,
|
||||
followupAutoApproveTimeoutMs,
|
||||
+ diagnosticsEnabled,
|
||||
} = await this.getState()
|
||||
|
||||
const telemetryKey = process.env.POSTHOG_API_KEY
|
||||
@@ -1489,7 +1491,7 @@ export class ClineProvider
|
||||
remoteBrowserHost,
|
||||
remoteBrowserEnabled: remoteBrowserEnabled ?? false,
|
||||
cachedChromeHostUrl: cachedChromeHostUrl,
|
||||
- writeDelayMs: writeDelayMs ?? 1000,
|
||||
+ writeDelayMs: writeDelayMs ?? DEFAULT_WRITE_DELAY_MS,
|
||||
terminalOutputLineLimit: terminalOutputLineLimit ?? 500,
|
||||
terminalShellIntegrationTimeout: terminalShellIntegrationTimeout ?? Terminal.defaultShellIntegrationTimeout,
|
||||
terminalShellIntegrationDisabled: terminalShellIntegrationDisabled ?? false,
|
||||
@@ -1555,6 +1557,7 @@ export class ClineProvider
|
||||
hasOpenedModeSelector: this.getGlobalState("hasOpenedModeSelector") ?? false,
|
||||
alwaysAllowFollowupQuestions: alwaysAllowFollowupQuestions ?? false,
|
||||
followupAutoApproveTimeoutMs: followupAutoApproveTimeoutMs ?? 60000,
|
||||
+ diagnosticsEnabled: diagnosticsEnabled ?? true,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1638,6 +1641,7 @@ export class ClineProvider
|
||||
alwaysAllowFollowupQuestions: stateValues.alwaysAllowFollowupQuestions ?? false,
|
||||
alwaysAllowUpdateTodoList: stateValues.alwaysAllowUpdateTodoList ?? false,
|
||||
followupAutoApproveTimeoutMs: stateValues.followupAutoApproveTimeoutMs ?? 60000,
|
||||
+ diagnosticsEnabled: stateValues.diagnosticsEnabled ?? true,
|
||||
allowedMaxRequests: stateValues.allowedMaxRequests,
|
||||
autoCondenseContext: stateValues.autoCondenseContext ?? true,
|
||||
autoCondenseContextPercent: stateValues.autoCondenseContextPercent ?? 100,
|
||||
@@ -1656,7 +1660,7 @@ export class ClineProvider
|
||||
remoteBrowserEnabled: stateValues.remoteBrowserEnabled ?? false,
|
||||
cachedChromeHostUrl: stateValues.cachedChromeHostUrl as string | undefined,
|
||||
fuzzyMatchThreshold: stateValues.fuzzyMatchThreshold ?? 1.0,
|
||||
- writeDelayMs: stateValues.writeDelayMs ?? 1000,
|
||||
+ writeDelayMs: stateValues.writeDelayMs ?? DEFAULT_WRITE_DELAY_MS,
|
||||
terminalOutputLineLimit: stateValues.terminalOutputLineLimit ?? 500,
|
||||
terminalShellIntegrationTimeout:
|
||||
stateValues.terminalShellIntegrationTimeout ?? Terminal.defaultShellIntegrationTimeout,
|
||||
diff --git a/src/core/webview/__tests__/ClineProvider.spec.ts b/src/core/webview/__tests__/ClineProvider.spec.ts
|
||||
index dd9ee12bfcb..1db9e4de6c7 100644
|
||||
--- a/src/core/webview/__tests__/ClineProvider.spec.ts
|
||||
+++ b/src/core/webview/__tests__/ClineProvider.spec.ts
|
||||
@@ -14,6 +14,7 @@ import { setTtsEnabled } from "../../../utils/tts"
|
||||
import { ContextProxy } from "../../config/ContextProxy"
|
||||
import { Task, TaskOptions } from "../../task/Task"
|
||||
import { safeWriteJson } from "../../../utils/safeWriteJson"
|
||||
+import { DEFAULT_WRITE_DELAY_MS } from "../../../shared/constants"
|
||||
|
||||
import { ClineProvider } from "../ClineProvider"
|
||||
|
||||
@@ -500,24 +501,30 @@ describe("ClineProvider", () => {
|
||||
alwaysAllowReadOnly: false,
|
||||
alwaysAllowReadOnlyOutsideWorkspace: false,
|
||||
alwaysAllowWrite: false,
|
||||
- codebaseIndexConfig: {
|
||||
- codebaseIndexEnabled: true,
|
||||
- codebaseIndexQdrantUrl: "",
|
||||
- codebaseIndexEmbedderProvider: "openai",
|
||||
- codebaseIndexEmbedderBaseUrl: "",
|
||||
- codebaseIndexEmbedderModelId: "",
|
||||
- },
|
||||
alwaysAllowWriteOutsideWorkspace: false,
|
||||
+ alwaysAllowWriteProtected: false,
|
||||
alwaysAllowExecute: false,
|
||||
alwaysAllowBrowser: false,
|
||||
alwaysAllowMcp: false,
|
||||
+ alwaysAllowModeSwitch: false,
|
||||
+ alwaysAllowSubtasks: false,
|
||||
+ alwaysAllowUpdateTodoList: false,
|
||||
+ allowedCommands: [],
|
||||
+ deniedCommands: [],
|
||||
+ allowedMaxRequests: 100,
|
||||
uriScheme: "vscode",
|
||||
soundEnabled: false,
|
||||
+ soundVolume: 0.5,
|
||||
ttsEnabled: false,
|
||||
+ ttsSpeed: 1.0,
|
||||
diffEnabled: false,
|
||||
enableCheckpoints: false,
|
||||
writeDelayMs: 1000,
|
||||
browserViewportSize: "900x600",
|
||||
+ browserToolEnabled: true,
|
||||
+ remoteBrowserEnabled: false,
|
||||
+ remoteBrowserHost: "",
|
||||
+ screenshotQuality: 0.8,
|
||||
fuzzyMatchThreshold: 1.0,
|
||||
mcpEnabled: true,
|
||||
enableMcpServerCreation: false,
|
||||
@@ -527,11 +534,23 @@ describe("ClineProvider", () => {
|
||||
experiments: experimentDefault,
|
||||
maxOpenTabsContext: 20,
|
||||
maxWorkspaceFiles: 200,
|
||||
- browserToolEnabled: true,
|
||||
+ maxReadFileLine: 500,
|
||||
+ maxConcurrentFileReads: 10,
|
||||
+ terminalOutputLineLimit: 1000,
|
||||
+ terminalShellIntegrationTimeout: 5000,
|
||||
+ terminalShellIntegrationDisabled: false,
|
||||
+ terminalCommandDelay: 100,
|
||||
+ terminalPowershellCounter: false,
|
||||
+ terminalZshClearEolMark: false,
|
||||
+ terminalZshOhMy: false,
|
||||
+ terminalZshP10k: false,
|
||||
+ terminalZdotdir: false,
|
||||
+ terminalCompressProgressBar: false,
|
||||
+ diagnosticsEnabled: true,
|
||||
+ language: "en",
|
||||
telemetrySetting: "unset",
|
||||
showRooIgnoredFiles: true,
|
||||
renderContext: "sidebar",
|
||||
- maxReadFileLine: 500,
|
||||
cloudUserInfo: null,
|
||||
organizationAllowList: ORGANIZATION_ALLOW_ALL,
|
||||
autoCondenseContext: true,
|
||||
@@ -540,6 +559,26 @@ describe("ClineProvider", () => {
|
||||
sharingEnabled: false,
|
||||
profileThresholds: {},
|
||||
hasOpenedModeSelector: false,
|
||||
+ // Add missing required properties
|
||||
+ currentApiConfigName: "test-config",
|
||||
+ listApiConfigMeta: [],
|
||||
+ pinnedApiConfigs: {},
|
||||
+ autoApprovalEnabled: false,
|
||||
+ alwaysApproveResubmit: false,
|
||||
+ customModePrompts: {},
|
||||
+ customSupportPrompts: {},
|
||||
+ modeApiConfigs: {},
|
||||
+ enhancementApiConfigId: "",
|
||||
+ condensingApiConfigId: "",
|
||||
+ customCondensingPrompt: "",
|
||||
+ codebaseIndexConfig: {
|
||||
+ codebaseIndexEnabled: true,
|
||||
+ codebaseIndexQdrantUrl: "",
|
||||
+ codebaseIndexEmbedderProvider: "openai",
|
||||
+ codebaseIndexEmbedderBaseUrl: "",
|
||||
+ codebaseIndexEmbedderModelId: "",
|
||||
+ },
|
||||
+ codebaseIndexModels: {},
|
||||
}
|
||||
|
||||
const message: ExtensionMessage = {
|
||||
diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts
|
||||
index 2efb2cbdff1..1bb76734db3 100644
|
||||
--- a/src/core/webview/webviewMessageHandler.ts
|
||||
+++ b/src/core/webview/webviewMessageHandler.ts
|
||||
@@ -1044,6 +1044,10 @@ export const webviewMessageHandler = async (
|
||||
await updateGlobalState("writeDelayMs", message.value)
|
||||
await provider.postStateToWebview()
|
||||
break
|
||||
+ case "diagnosticsEnabled":
|
||||
+ await updateGlobalState("diagnosticsEnabled", message.bool ?? true)
|
||||
+ await provider.postStateToWebview()
|
||||
+ break
|
||||
case "terminalOutputLineLimit":
|
||||
await updateGlobalState("terminalOutputLineLimit", message.value)
|
||||
await provider.postStateToWebview()
|
||||
diff --git a/src/integrations/editor/DiffViewProvider.ts b/src/integrations/editor/DiffViewProvider.ts
|
||||
index 225e076297e..32eb7bfcfd7 100644
|
||||
--- a/src/integrations/editor/DiffViewProvider.ts
|
||||
+++ b/src/integrations/editor/DiffViewProvider.ts
|
||||
@@ -4,6 +4,7 @@ import * as fs from "fs/promises"
|
||||
import * as diff from "diff"
|
||||
import stripBom from "strip-bom"
|
||||
import { XMLBuilder } from "fast-xml-parser"
|
||||
+import delay from "delay"
|
||||
|
||||
import { createDirectoriesForFile } from "../../utils/fs"
|
||||
import { arePathsEqual, getReadablePath } from "../../utils/path"
|
||||
@@ -11,6 +12,7 @@ import { formatResponse } from "../../core/prompts/responses"
|
||||
import { diagnosticsToProblemsString, getNewDiagnostics } from "../diagnostics"
|
||||
import { ClineSayTool } from "../../shared/ExtensionMessage"
|
||||
import { Task } from "../../core/task/Task"
|
||||
+import { DEFAULT_WRITE_DELAY_MS } from "../../shared/constants"
|
||||
|
||||
import { DecorationController } from "./DecorationController"
|
||||
|
||||
@@ -179,7 +181,7 @@ export class DiffViewProvider {
|
||||
}
|
||||
}
|
||||
|
||||
- async saveChanges(): Promise<{
|
||||
+ async saveChanges(diagnosticsEnabled: boolean = true, writeDelayMs: number = DEFAULT_WRITE_DELAY_MS): Promise<{
|
||||
newProblemsMessage: string | undefined
|
||||
userEdits: string | undefined
|
||||
finalContent: string | undefined
|
||||
@@ -214,18 +216,35 @@ export class DiffViewProvider {
|
||||
// and can address them accordingly. If problems don't change immediately after
|
||||
// applying a fix, won't be notified, which is generally fine since the
|
||||
// initial fix is usually correct and it may just take time for linters to catch up.
|
||||
- const postDiagnostics = vscode.languages.getDiagnostics()
|
||||
-
|
||||
- const newProblems = await diagnosticsToProblemsString(
|
||||
- getNewDiagnostics(this.preDiagnostics, postDiagnostics),
|
||||
- [
|
||||
- vscode.DiagnosticSeverity.Error, // only including errors since warnings can be distracting (if user wants to fix warnings they can use the @problems mention)
|
||||
- ],
|
||||
- this.cwd,
|
||||
- ) // Will be empty string if no errors.
|
||||
-
|
||||
- const newProblemsMessage =
|
||||
- newProblems.length > 0 ? `\n\nNew problems detected after saving the file:\n${newProblems}` : ""
|
||||
+
|
||||
+ let newProblemsMessage = ""
|
||||
+
|
||||
+ if (diagnosticsEnabled) {
|
||||
+ // Add configurable delay to allow linters time to process and clean up issues
|
||||
+ // like unused imports (especially important for Go and other languages)
|
||||
+ // Ensure delay is non-negative
|
||||
+ const safeDelayMs = Math.max(0, writeDelayMs)
|
||||
+
|
||||
+ try {
|
||||
+ await delay(safeDelayMs)
|
||||
+ } catch (error) {
|
||||
+ // Log error but continue - delay failure shouldn't break the save operation
|
||||
+ console.warn(`Failed to apply write delay: ${error}`)
|
||||
+ }
|
||||
+
|
||||
+ const postDiagnostics = vscode.languages.getDiagnostics()
|
||||
+
|
||||
+ const newProblems = await diagnosticsToProblemsString(
|
||||
+ getNewDiagnostics(this.preDiagnostics, postDiagnostics),
|
||||
+ [
|
||||
+ vscode.DiagnosticSeverity.Error, // only including errors since warnings can be distracting (if user wants to fix warnings they can use the @problems mention)
|
||||
+ ],
|
||||
+ this.cwd,
|
||||
+ ) // Will be empty string if no errors.
|
||||
+
|
||||
+ newProblemsMessage =
|
||||
+ newProblems.length > 0 ? `\n\nNew problems detected after saving the file:\n${newProblems}` : ""
|
||||
+ }
|
||||
|
||||
// If the edited content has different EOL characters, we don't want to
|
||||
// show a diff with all the EOL differences.
|
||||
diff --git a/src/integrations/editor/__tests__/DiffViewProvider.spec.ts b/src/integrations/editor/__tests__/DiffViewProvider.spec.ts
|
||||
index ad1950345bd..a4aded95bb9 100644
|
||||
--- a/src/integrations/editor/__tests__/DiffViewProvider.spec.ts
|
||||
+++ b/src/integrations/editor/__tests__/DiffViewProvider.spec.ts
|
||||
@@ -1,6 +1,12 @@
|
||||
import { DiffViewProvider, DIFF_VIEW_URI_SCHEME, DIFF_VIEW_LABEL_CHANGES } from "../DiffViewProvider"
|
||||
import * as vscode from "vscode"
|
||||
import * as path from "path"
|
||||
+import delay from "delay"
|
||||
+
|
||||
+// Mock delay
|
||||
+vi.mock("delay", () => ({
|
||||
+ default: vi.fn().mockResolvedValue(undefined),
|
||||
+}))
|
||||
|
||||
// Mock fs/promises
|
||||
vi.mock("fs/promises", () => ({
|
||||
@@ -45,6 +51,12 @@ vi.mock("vscode", () => ({
|
||||
languages: {
|
||||
getDiagnostics: vi.fn(() => []),
|
||||
},
|
||||
+ DiagnosticSeverity: {
|
||||
+ Error: 0,
|
||||
+ Warning: 1,
|
||||
+ Information: 2,
|
||||
+ Hint: 3,
|
||||
+ },
|
||||
WorkspaceEdit: vi.fn().mockImplementation(() => ({
|
||||
replace: vi.fn(),
|
||||
delete: vi.fn(),
|
||||
@@ -327,4 +339,83 @@ describe("DiffViewProvider", () => {
|
||||
).toBeUndefined()
|
||||
})
|
||||
})
|
||||
+
|
||||
+ describe("saveChanges method with diagnostic settings", () => {
|
||||
+ beforeEach(() => {
|
||||
+ // Setup common mocks for saveChanges tests
|
||||
+ ;(diffViewProvider as any).relPath = "test.ts"
|
||||
+ ;(diffViewProvider as any).newContent = "new content"
|
||||
+ ;(diffViewProvider as any).activeDiffEditor = {
|
||||
+ document: {
|
||||
+ getText: vi.fn().mockReturnValue("new content"),
|
||||
+ isDirty: false,
|
||||
+ save: vi.fn().mockResolvedValue(undefined),
|
||||
+ },
|
||||
+ }
|
||||
+ ;(diffViewProvider as any).preDiagnostics = []
|
||||
+
|
||||
+ // Mock vscode functions
|
||||
+ vi.mocked(vscode.window.showTextDocument).mockResolvedValue({} as any)
|
||||
+ vi.mocked(vscode.languages.getDiagnostics).mockReturnValue([])
|
||||
+ })
|
||||
+
|
||||
+ it("should apply diagnostic delay when diagnosticsEnabled is true", async () => {
|
||||
+ const mockDelay = vi.mocked(delay)
|
||||
+ mockDelay.mockClear()
|
||||
+
|
||||
+ // Mock closeAllDiffViews
|
||||
+ ;(diffViewProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined)
|
||||
+
|
||||
+ const result = await diffViewProvider.saveChanges(true, 3000)
|
||||
+
|
||||
+ // Verify delay was called with correct duration
|
||||
+ expect(mockDelay).toHaveBeenCalledWith(3000)
|
||||
+ expect(vscode.languages.getDiagnostics).toHaveBeenCalled()
|
||||
+ expect(result.newProblemsMessage).toBe("")
|
||||
+ })
|
||||
+
|
||||
+ it("should skip diagnostics when diagnosticsEnabled is false", async () => {
|
||||
+ const mockDelay = vi.mocked(delay)
|
||||
+ mockDelay.mockClear()
|
||||
+
|
||||
+ // Mock closeAllDiffViews
|
||||
+ ;(diffViewProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined)
|
||||
+
|
||||
+ const result = await diffViewProvider.saveChanges(false, 2000)
|
||||
+
|
||||
+ // Verify delay was NOT called and diagnostics were NOT checked
|
||||
+ expect(mockDelay).not.toHaveBeenCalled()
|
||||
+ expect(vscode.languages.getDiagnostics).not.toHaveBeenCalled()
|
||||
+ expect(result.newProblemsMessage).toBe("")
|
||||
+ })
|
||||
+
|
||||
+ it("should use default values when no parameters provided", async () => {
|
||||
+ const mockDelay = vi.mocked(delay)
|
||||
+ mockDelay.mockClear()
|
||||
+
|
||||
+ // Mock closeAllDiffViews
|
||||
+ ;(diffViewProvider as any).closeAllDiffViews = vi.fn().mockResolvedValue(undefined)
|
||||
+
|
||||
+ const result = await diffViewProvider.saveChanges()
|
||||
+
|
||||
+ // Verify default behavior (enabled=true, delay=2000ms)
|
||||
+ expect(mockDelay).toHaveBeenCalledWith(1000)
|
||||
+ expect(vscode.languages.getDiagnostics).toHaveBeenCalled()
|
||||
+ expect(result.newProblemsMessage).toBe("")
|
||||
+ })
|
||||
+
|
||||
+ it("should handle custom delay values", async () => {
|
||||
+ const mockDelay = vi.mocked(delay)
|
||||
+ mockDelay.mockClear()
|
||||
+
|
||||
+ // Mock closeAllDiffViews
|
||||
+ ;(diffViewProvider as any).closeAllDiffViews = vi.
|
||||
1
.roo/temp/pr-5863/pr-metadata.json
Normal file
1
.roo/temp/pr-5863/pr-metadata.json
Normal file
File diff suppressed because one or more lines are too long
20
.roo/temp/pr-5863/review-context.json
Normal file
20
.roo/temp/pr-5863/review-context.json
Normal file
|
|
@ -0,0 +1,20 @@
|
|||
{
|
||||
"prNumber": "5863",
|
||||
"repository": "RooCodeInc/Roo-Code",
|
||||
"reviewStartTime": "2025-01-18T18:52:44Z",
|
||||
"calledByMode": null,
|
||||
"prMetadata": {},
|
||||
"linkedIssue": {},
|
||||
"existingComments": [],
|
||||
"existingReviews": [],
|
||||
"filesChanged": [],
|
||||
"delegatedTasks": [],
|
||||
"findings": {
|
||||
"critical": [],
|
||||
"patterns": [],
|
||||
"redundancy": [],
|
||||
"architecture": [],
|
||||
"tests": []
|
||||
},
|
||||
"reviewStatus": "initialized"
|
||||
}
|
||||
|
|
@ -384,7 +384,7 @@ describe("writeToFileTool", () => {
|
|||
userEdits: userEditsValue,
|
||||
finalContent: "modified content",
|
||||
})
|
||||
// Set the userEdits property on the diffViewProvider mock to simulate user edits
|
||||
// Manually set the property on the mock instance because the original saveChanges is not called
|
||||
mockCline.diffViewProvider.userEdits = userEditsValue
|
||||
|
||||
await executeWriteFileTool({}, { fileExists: true })
|
||||
|
|
|
|||
|
|
@ -501,30 +501,24 @@ describe("ClineProvider", () => {
|
|||
alwaysAllowReadOnly: false,
|
||||
alwaysAllowReadOnlyOutsideWorkspace: false,
|
||||
alwaysAllowWrite: false,
|
||||
codebaseIndexConfig: {
|
||||
codebaseIndexEnabled: true,
|
||||
codebaseIndexQdrantUrl: "",
|
||||
codebaseIndexEmbedderProvider: "openai",
|
||||
codebaseIndexEmbedderBaseUrl: "",
|
||||
codebaseIndexEmbedderModelId: "",
|
||||
},
|
||||
alwaysAllowWriteOutsideWorkspace: false,
|
||||
alwaysAllowWriteProtected: false,
|
||||
alwaysAllowExecute: false,
|
||||
alwaysAllowBrowser: false,
|
||||
alwaysAllowMcp: false,
|
||||
alwaysAllowModeSwitch: false,
|
||||
alwaysAllowSubtasks: false,
|
||||
alwaysAllowUpdateTodoList: false,
|
||||
allowedCommands: [],
|
||||
deniedCommands: [],
|
||||
allowedMaxRequests: 100,
|
||||
uriScheme: "vscode",
|
||||
soundEnabled: false,
|
||||
soundVolume: 0.5,
|
||||
ttsEnabled: false,
|
||||
ttsSpeed: 1.0,
|
||||
diffEnabled: false,
|
||||
enableCheckpoints: false,
|
||||
writeDelayMs: 1000,
|
||||
browserViewportSize: "900x600",
|
||||
browserToolEnabled: true,
|
||||
remoteBrowserEnabled: false,
|
||||
remoteBrowserHost: "",
|
||||
screenshotQuality: 0.8,
|
||||
fuzzyMatchThreshold: 1.0,
|
||||
mcpEnabled: true,
|
||||
enableMcpServerCreation: false,
|
||||
|
|
@ -534,23 +528,11 @@ describe("ClineProvider", () => {
|
|||
experiments: experimentDefault,
|
||||
maxOpenTabsContext: 20,
|
||||
maxWorkspaceFiles: 200,
|
||||
maxReadFileLine: 500,
|
||||
maxConcurrentFileReads: 10,
|
||||
terminalOutputLineLimit: 1000,
|
||||
terminalShellIntegrationTimeout: 5000,
|
||||
terminalShellIntegrationDisabled: false,
|
||||
terminalCommandDelay: 100,
|
||||
terminalPowershellCounter: false,
|
||||
terminalZshClearEolMark: false,
|
||||
terminalZshOhMy: false,
|
||||
terminalZshP10k: false,
|
||||
terminalZdotdir: false,
|
||||
terminalCompressProgressBar: false,
|
||||
diagnosticsEnabled: true,
|
||||
language: "en",
|
||||
browserToolEnabled: true,
|
||||
telemetrySetting: "unset",
|
||||
showRooIgnoredFiles: true,
|
||||
renderContext: "sidebar",
|
||||
maxReadFileLine: 500,
|
||||
cloudUserInfo: null,
|
||||
organizationAllowList: ORGANIZATION_ALLOW_ALL,
|
||||
autoCondenseContext: true,
|
||||
|
|
@ -559,26 +541,7 @@ describe("ClineProvider", () => {
|
|||
sharingEnabled: false,
|
||||
profileThresholds: {},
|
||||
hasOpenedModeSelector: false,
|
||||
// Add missing required properties
|
||||
currentApiConfigName: "test-config",
|
||||
listApiConfigMeta: [],
|
||||
pinnedApiConfigs: {},
|
||||
autoApprovalEnabled: false,
|
||||
alwaysApproveResubmit: false,
|
||||
customModePrompts: {},
|
||||
customSupportPrompts: {},
|
||||
modeApiConfigs: {},
|
||||
enhancementApiConfigId: "",
|
||||
condensingApiConfigId: "",
|
||||
customCondensingPrompt: "",
|
||||
codebaseIndexConfig: {
|
||||
codebaseIndexEnabled: true,
|
||||
codebaseIndexQdrantUrl: "",
|
||||
codebaseIndexEmbedderProvider: "openai",
|
||||
codebaseIndexEmbedderBaseUrl: "",
|
||||
codebaseIndexEmbedderModelId: "",
|
||||
},
|
||||
codebaseIndexModels: {},
|
||||
diagnosticsEnabled: true,
|
||||
}
|
||||
|
||||
const message: ExtensionMessage = {
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue