mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
refactor: use existing writeDelayMs instead of diagnosticsDelayMs
- Remove diagnosticsDelayMs setting in favor of existing writeDelayMs - Add min(0) validation for writeDelayMs in global settings schema - Add error handling around delay function calls in DiffViewProvider - Create DEFAULT_WRITE_DELAY_MS constant (1000ms) to replace repeated defaults - Update all tool files to pass writeDelayMs instead of diagnosticsDelayMs - Remove diagnosticsDelayMs from webview message handlers and types - Update test files to use writeDelayMs instead of diagnosticsDelayMs This refactoring consolidates diagnostic delay functionality to use the existing writeDelayMs setting as requested in PR feedback.
This commit is contained in:
parent
e1a105d313
commit
0d49afda13
15 changed files with 86 additions and 37 deletions
|
|
@ -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,7 +86,6 @@ export const globalSettingsSchema = z.object({
|
|||
terminalZdotdir: z.boolean().optional(),
|
||||
terminalCompressProgressBar: z.boolean().optional(),
|
||||
|
||||
diagnosticsDelayMs: z.number().optional(),
|
||||
diagnosticsEnabled: z.boolean().optional(),
|
||||
|
||||
rateLimitSeconds: z.number().optional(),
|
||||
|
|
@ -227,7 +226,6 @@ export const EVALS_SETTINGS: RooCodeSettings = {
|
|||
terminalCompressProgressBar: true,
|
||||
terminalShellIntegrationDisabled: true,
|
||||
|
||||
diagnosticsDelayMs: 2000,
|
||||
diagnosticsEnabled: true,
|
||||
|
||||
diffEnabled: true,
|
||||
|
|
|
|||
|
|
@ -75,7 +75,7 @@ describe("insertContentTool", () => {
|
|||
deref: vi.fn().mockReturnValue({
|
||||
getState: vi.fn().mockResolvedValue({
|
||||
diagnosticsEnabled: true,
|
||||
diagnosticsDelayMs: 2000,
|
||||
writeDelayMs: 1000,
|
||||
}),
|
||||
}),
|
||||
},
|
||||
|
|
|
|||
|
|
@ -136,7 +136,7 @@ describe("writeToFileTool", () => {
|
|||
deref: vi.fn().mockReturnValue({
|
||||
getState: vi.fn().mockResolvedValue({
|
||||
diagnosticsEnabled: true,
|
||||
diagnosticsDelayMs: 2000,
|
||||
writeDelayMs: 1000,
|
||||
}),
|
||||
}),
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
@ -173,8 +174,8 @@ export async function applyDiffToolLegacy(
|
|||
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)
|
||||
const writeDelayMs = state?.writeDelayMs ?? DEFAULT_WRITE_DELAY_MS
|
||||
await cline.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs)
|
||||
|
||||
// Track file edit operation
|
||||
if (relPath) {
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ import { ClineSayTool } from "../../shared/ExtensionMessage"
|
|||
import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
|
||||
import { fileExistsAtPath } from "../../utils/fs"
|
||||
import { insertGroups } from "../diff/insert-groups"
|
||||
import { DEFAULT_WRITE_DELAY_MS } from "../../shared/constants"
|
||||
|
||||
export async function insertContentTool(
|
||||
cline: Task,
|
||||
|
|
@ -158,8 +159,8 @@ export async function insertContentTool(
|
|||
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)
|
||||
const writeDelayMs = state?.writeDelayMs ?? DEFAULT_WRITE_DELAY_MS
|
||||
await cline.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs)
|
||||
|
||||
// Track file edit operation
|
||||
if (relPath) {
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
@ -556,8 +557,8 @@ ${errorDetails ? `\nTechnical details:\n${errorDetails}\n` : ""}
|
|||
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)
|
||||
const writeDelayMs = state?.writeDelayMs ?? DEFAULT_WRITE_DELAY_MS
|
||||
await cline.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs)
|
||||
|
||||
// Track file edit operation
|
||||
await cline.fileContextTracker.trackFileContext(relPath, "roo_edited" as RecordSource)
|
||||
|
|
|
|||
|
|
@ -11,6 +11,7 @@ import { ClineSayTool } from "../../shared/ExtensionMessage"
|
|||
import { getReadablePath } from "../../utils/path"
|
||||
import { fileExistsAtPath } from "../../utils/fs"
|
||||
import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
|
||||
import { DEFAULT_WRITE_DELAY_MS } from "../../shared/constants"
|
||||
|
||||
/**
|
||||
* Tool for performing search and replace operations on files
|
||||
|
|
@ -230,8 +231,8 @@ export async function searchAndReplaceTool(
|
|||
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)
|
||||
const writeDelayMs = state?.writeDelayMs ?? DEFAULT_WRITE_DELAY_MS
|
||||
await cline.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs)
|
||||
|
||||
// Track file edit operation
|
||||
if (relPath) {
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ import { getReadablePath } from "../../utils/path"
|
|||
import { isPathOutsideWorkspace } from "../../utils/pathUtils"
|
||||
import { detectCodeOmission } from "../../integrations/editor/detect-omission"
|
||||
import { unescapeHtmlEntities } from "../../utils/text-normalization"
|
||||
import { DEFAULT_WRITE_DELAY_MS } from "../../shared/constants"
|
||||
|
||||
export async function writeToFileTool(
|
||||
cline: Task,
|
||||
|
|
@ -216,8 +217,8 @@ export async function writeToFileTool(
|
|||
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)
|
||||
const writeDelayMs = state?.writeDelayMs ?? DEFAULT_WRITE_DELAY_MS
|
||||
await cline.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs)
|
||||
|
||||
// Track file edit operation
|
||||
if (relPath) {
|
||||
|
|
|
|||
|
|
@ -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,7 +1437,6 @@ export class ClineProvider
|
|||
profileThresholds,
|
||||
alwaysAllowFollowupQuestions,
|
||||
followupAutoApproveTimeoutMs,
|
||||
diagnosticsDelayMs,
|
||||
diagnosticsEnabled,
|
||||
} = await this.getState()
|
||||
|
||||
|
|
@ -1491,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,
|
||||
|
|
@ -1557,7 +1557,6 @@ export class ClineProvider
|
|||
hasOpenedModeSelector: this.getGlobalState("hasOpenedModeSelector") ?? false,
|
||||
alwaysAllowFollowupQuestions: alwaysAllowFollowupQuestions ?? false,
|
||||
followupAutoApproveTimeoutMs: followupAutoApproveTimeoutMs ?? 60000,
|
||||
diagnosticsDelayMs: diagnosticsDelayMs ?? 2000,
|
||||
diagnosticsEnabled: diagnosticsEnabled ?? true,
|
||||
}
|
||||
}
|
||||
|
|
@ -1642,7 +1641,6 @@ 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,
|
||||
|
|
@ -1662,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,
|
||||
|
|
|
|||
|
|
@ -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: 0,
|
||||
terminalZshClearEolMark: false,
|
||||
terminalZshOhMy: false,
|
||||
terminalZshP10k: false,
|
||||
terminalZdotdir: "",
|
||||
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 = {
|
||||
|
|
|
|||
|
|
@ -1044,10 +1044,6 @@ 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()
|
||||
|
|
|
|||
|
|
@ -12,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"
|
||||
|
||||
|
|
@ -180,7 +181,7 @@ export class DiffViewProvider {
|
|||
}
|
||||
}
|
||||
|
||||
async saveChanges(diagnosticsEnabled: boolean = true, diagnosticsDelayMs: number = 2000): Promise<{
|
||||
async saveChanges(diagnosticsEnabled: boolean = true, writeDelayMs: number = DEFAULT_WRITE_DELAY_MS): Promise<{
|
||||
newProblemsMessage: string | undefined
|
||||
userEdits: string | undefined
|
||||
finalContent: string | undefined
|
||||
|
|
@ -221,7 +222,15 @@ export class DiffViewProvider {
|
|||
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)
|
||||
// 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()
|
||||
|
||||
|
|
|
|||
|
|
@ -214,7 +214,6 @@ export type ExtensionState = Pick<
|
|||
| "terminalZshP10k"
|
||||
| "terminalZdotdir"
|
||||
| "terminalCompressProgressBar"
|
||||
| "diagnosticsDelayMs"
|
||||
| "diagnosticsEnabled"
|
||||
| "diffEnabled"
|
||||
| "fuzzyMatchThreshold"
|
||||
|
|
|
|||
|
|
@ -107,7 +107,6 @@ export interface WebviewMessage {
|
|||
| "updateMcpTimeout"
|
||||
| "fuzzyMatchThreshold"
|
||||
| "writeDelayMs"
|
||||
| "diagnosticsDelayMs"
|
||||
| "diagnosticsEnabled"
|
||||
| "enhancePrompt"
|
||||
| "enhancedPrompt"
|
||||
|
|
|
|||
6
src/shared/constants.ts
Normal file
6
src/shared/constants.ts
Normal file
|
|
@ -0,0 +1,6 @@
|
|||
/**
|
||||
* Default delay in milliseconds after writes to allow diagnostics to detect potential problems.
|
||||
* This delay is particularly important for Go and other languages where tools like goimports
|
||||
* need time to automatically clean up unused imports.
|
||||
*/
|
||||
export const DEFAULT_WRITE_DELAY_MS = 1000
|
||||
Loading…
Add table
Reference in a new issue