Skip to content
Merged
7,638 changes: 7,638 additions & 0 deletions package-lock.json

Large diffs are not rendered by default.

6 changes: 6 additions & 0 deletions packages/types/src/global-settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,9 @@ export const globalSettingsSchema = z.object({
terminalZdotdir: z.boolean().optional(),
terminalCompressProgressBar: z.boolean().optional(),

diagnosticsDelayMs: z.number().optional(),
Copy link
Contributor

Choose a reason for hiding this comment

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

New diagnostic settings (diagnosticsDelayMs, diagnosticsEnabled) added to the schema. Consider validating that diagnosticsDelayMs is non-negative to avoid invalid delay values.

Suggested change
diagnosticsDelayMs: z.number().optional(),
diagnosticsDelayMs: z.number().min(0).optional(),

diagnosticsEnabled: z.boolean().optional(),

rateLimitSeconds: z.number().optional(),
diffEnabled: z.boolean().optional(),
fuzzyMatchThreshold: z.number().optional(),
Expand Down Expand Up @@ -224,6 +227,9 @@ export const EVALS_SETTINGS: RooCodeSettings = {
terminalCompressProgressBar: true,
terminalShellIntegrationDisabled: true,

diagnosticsDelayMs: 2000,
diagnosticsEnabled: true,

diffEnabled: true,
fuzzyMatchThreshold: 1,

Expand Down
6 changes: 5 additions & 1 deletion src/core/tools/applyDiffTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,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 diagnosticsDelayMs = state?.diagnosticsDelayMs ?? 2000
await cline.diffViewProvider.saveChanges(diagnosticsEnabled, diagnosticsDelayMs)

// Track file edit operation
if (relPath) {
Expand Down
6 changes: 5 additions & 1 deletion src/core/tools/insertContentTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,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 diagnosticsDelayMs = state?.diagnosticsDelayMs ?? 2000
await cline.diffViewProvider.saveChanges(diagnosticsEnabled, diagnosticsDelayMs)

// Track file edit operation
if (relPath) {
Expand Down
6 changes: 5 additions & 1 deletion src/core/tools/multiApplyDiffTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,11 @@ ${errorDetails ? `\nTechnical details:\n${errorDetails}\n` : ""}
}

// 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 diagnosticsDelayMs = state?.diagnosticsDelayMs ?? 2000
await cline.diffViewProvider.saveChanges(diagnosticsEnabled, diagnosticsDelayMs)

// Track file edit operation
await cline.fileContextTracker.trackFileContext(relPath, "roo_edited" as RecordSource)
Expand Down
6 changes: 5 additions & 1 deletion src/core/tools/searchAndReplaceTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,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 diagnosticsDelayMs = state?.diagnosticsDelayMs ?? 2000
await cline.diffViewProvider.saveChanges(diagnosticsEnabled, diagnosticsDelayMs)

// Track file edit operation
if (relPath) {
Expand Down
6 changes: 5 additions & 1 deletion src/core/tools/writeToFileTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,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 diagnosticsDelayMs = state?.diagnosticsDelayMs ?? 2000
await cline.diffViewProvider.saveChanges(diagnosticsEnabled, diagnosticsDelayMs)

// Track file edit operation
if (relPath) {
Expand Down
6 changes: 6 additions & 0 deletions src/core/webview/ClineProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1436,6 +1436,8 @@ export class ClineProvider
profileThresholds,
alwaysAllowFollowupQuestions,
followupAutoApproveTimeoutMs,
diagnosticsDelayMs,
diagnosticsEnabled,
} = await this.getState()

const telemetryKey = process.env.POSTHOG_API_KEY
Expand Down Expand Up @@ -1555,6 +1557,8 @@ export class ClineProvider
hasOpenedModeSelector: this.getGlobalState("hasOpenedModeSelector") ?? false,
alwaysAllowFollowupQuestions: alwaysAllowFollowupQuestions ?? false,
followupAutoApproveTimeoutMs: followupAutoApproveTimeoutMs ?? 60000,
diagnosticsDelayMs: diagnosticsDelayMs ?? 2000,
diagnosticsEnabled: diagnosticsEnabled ?? true,
}
}

Expand Down Expand Up @@ -1638,6 +1642,8 @@ export class ClineProvider
alwaysAllowFollowupQuestions: stateValues.alwaysAllowFollowupQuestions ?? false,
alwaysAllowUpdateTodoList: stateValues.alwaysAllowUpdateTodoList ?? false,
followupAutoApproveTimeoutMs: stateValues.followupAutoApproveTimeoutMs ?? 60000,
diagnosticsDelayMs: stateValues.diagnosticsDelayMs ?? 2000,
diagnosticsEnabled: stateValues.diagnosticsEnabled ?? true,
allowedMaxRequests: stateValues.allowedMaxRequests,
autoCondenseContext: stateValues.autoCondenseContext ?? true,
autoCondenseContextPercent: stateValues.autoCondenseContextPercent ?? 100,
Expand Down
8 changes: 8 additions & 0 deletions src/core/webview/webviewMessageHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1044,6 +1044,14 @@ export const webviewMessageHandler = async (
await updateGlobalState("writeDelayMs", message.value)
await provider.postStateToWebview()
break
case "diagnosticsDelayMs":
await updateGlobalState("diagnosticsDelayMs", 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()
Expand Down
36 changes: 23 additions & 13 deletions src/integrations/editor/DiffViewProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -179,7 +180,7 @@ export class DiffViewProvider {
}
}

async saveChanges(): Promise<{
async saveChanges(diagnosticsEnabled: boolean = true, diagnosticsDelayMs: number = 2000): Promise<{
newProblemsMessage: string | undefined
userEdits: string | undefined
finalContent: string | undefined
Expand Down Expand Up @@ -214,18 +215,27 @@ 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)
await delay(diagnosticsDelayMs)

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.
Expand Down
91 changes: 91 additions & 0 deletions src/integrations/editor/__tests__/DiffViewProvider.spec.ts
Original file line number Diff line number Diff line change
@@ -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", () => ({
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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)
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment mentions a delay of 2000ms (// Verify default behavior (enabled=true, delay=2000ms)), but the expectation on line 402 now uses 1000ms. Please update the comment to match the new behavior.

Suggested change
// Verify default behavior (enabled=true, delay=2000ms)
// Verify default behavior (enabled=true, delay=1000ms)

expect(mockDelay).toHaveBeenCalledWith(2000)
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.fn().mockResolvedValue(undefined)

const result = await diffViewProvider.saveChanges(true, 5000)

// Verify custom delay was used
expect(mockDelay).toHaveBeenCalledWith(5000)
expect(vscode.languages.getDiagnostics).toHaveBeenCalled()
})
})
})
2 changes: 2 additions & 0 deletions src/shared/ExtensionMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,8 @@ export type ExtensionState = Pick<
| "terminalZshP10k"
| "terminalZdotdir"
| "terminalCompressProgressBar"
| "diagnosticsDelayMs"
| "diagnosticsEnabled"
| "diffEnabled"
| "fuzzyMatchThreshold"
// | "experiments" // Optional in GlobalSettings, required here.
Expand Down
2 changes: 2 additions & 0 deletions src/shared/WebviewMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ export interface WebviewMessage {
| "updateMcpTimeout"
| "fuzzyMatchThreshold"
| "writeDelayMs"
| "diagnosticsDelayMs"
| "diagnosticsEnabled"
| "enhancePrompt"
| "enhancedPrompt"
| "draggedImages"
Expand Down
Loading