mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
feat: add configurable delay for Go diagnostics to prevent premature error reporting
- Add diagnosticsDelayMs setting (default: 2000ms) to allow linters time to process - Add diagnosticsEnabled setting to optionally disable diagnostic checking entirely - Update DiffViewProvider.saveChanges() to use configurable delay before checking diagnostics - Update all tool files (writeToFile, searchAndReplace, insertContent, applyDiff, multiApplyDiff) to pass diagnostic settings - Add comprehensive tests for new diagnostic functionality - Fixes issue where Go diagnostics errors were submitted to LLM before linter could clean up unused imports Resolves #5859
This commit is contained in:
parent
a28d50e20c
commit
87e7b3eba2
11 changed files with 149 additions and 16 deletions
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
|
|
@ -1436,6 +1436,8 @@ export class ClineProvider
|
|||
profileThresholds,
|
||||
alwaysAllowFollowupQuestions,
|
||||
followupAutoApproveTimeoutMs,
|
||||
diagnosticsDelayMs,
|
||||
diagnosticsEnabled,
|
||||
} = await this.getState()
|
||||
|
||||
const telemetryKey = process.env.POSTHOG_API_KEY
|
||||
|
|
@ -1555,6 +1557,8 @@ export class ClineProvider
|
|||
hasOpenedModeSelector: this.getGlobalState("hasOpenedModeSelector") ?? false,
|
||||
alwaysAllowFollowupQuestions: alwaysAllowFollowupQuestions ?? false,
|
||||
followupAutoApproveTimeoutMs: followupAutoApproveTimeoutMs ?? 60000,
|
||||
diagnosticsDelayMs: diagnosticsDelayMs ?? 2000,
|
||||
diagnosticsEnabled: diagnosticsEnabled ?? true,
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -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,
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
@ -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
|
||||
|
|
@ -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()
|
||||
|
||||
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.
|
||||
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}` : ""
|
||||
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.
|
||||
|
|
|
|||
|
|
@ -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", () => ({
|
||||
|
|
@ -327,4 +333,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(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()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -243,6 +243,8 @@ export type ExtensionState = Pick<
|
|||
taskHistory: HistoryItem[]
|
||||
|
||||
writeDelayMs: number
|
||||
diagnosticsDelayMs: number
|
||||
diagnosticsEnabled: boolean
|
||||
requestDelaySeconds: number
|
||||
|
||||
enableCheckpoints: boolean
|
||||
|
|
|
|||
|
|
@ -107,6 +107,8 @@ export interface WebviewMessage {
|
|||
| "updateMcpTimeout"
|
||||
| "fuzzyMatchThreshold"
|
||||
| "writeDelayMs"
|
||||
| "diagnosticsDelayMs"
|
||||
| "diagnosticsEnabled"
|
||||
| "enhancePrompt"
|
||||
| "enhancedPrompt"
|
||||
| "draggedImages"
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue