fix: prevent TextEditor disposed warnings by adding proper disposal checks

- Add isDisposed flag and dispose() method to DecorationController
- Add isEditorValid() checks before accessing TextEditor methods
- Properly dispose decoration controllers in DiffViewProvider.reset()
- Handle test environments where visibleTextEditors may be empty

Fixes #5954
This commit is contained in:
Roo Code 2025-07-19 18:54:16 +00:00
parent 37300ef0c5
commit 5afee2a6eb
2 changed files with 82 additions and 10 deletions

View file

@ -19,6 +19,7 @@ export class DecorationController {
private decorationType: DecorationType
private editor: vscode.TextEditor
private ranges: vscode.Range[] = []
private isDisposed: boolean = false
constructor(decorationType: DecorationType, editor: vscode.TextEditor) {
this.decorationType = decorationType
@ -35,8 +36,8 @@ export class DecorationController {
}
addLines(startIndex: number, numLines: number) {
// Guard against invalid inputs
if (startIndex < 0 || numLines <= 0) {
// Guard against invalid inputs or disposed editor
if (startIndex < 0 || numLines <= 0 || this.isDisposed || !this.isEditorValid()) {
return
}
@ -52,11 +53,18 @@ export class DecorationController {
}
clear() {
if (this.isDisposed || !this.isEditorValid()) {
return
}
this.ranges = []
this.editor.setDecorations(this.getDecoration(), this.ranges)
}
updateOverlayAfterLine(line: number, totalLines: number) {
if (this.isDisposed || !this.isEditorValid()) {
return
}
// Remove any existing ranges that start at or after the current line
this.ranges = this.ranges.filter((range) => range.end.line < line)
@ -75,7 +83,39 @@ export class DecorationController {
}
setActiveLine(line: number) {
if (this.isDisposed || !this.isEditorValid()) {
return
}
this.ranges = [new vscode.Range(line, 0, line, Number.MAX_SAFE_INTEGER)]
this.editor.setDecorations(this.getDecoration(), this.ranges)
}
dispose() {
this.isDisposed = true
// Clear decorations before disposing
if (this.isEditorValid()) {
this.editor.setDecorations(this.getDecoration(), [])
}
}
private isEditorValid(): boolean {
// Check if the editor is still valid by verifying it exists in visible editors
// and its document hasn't been closed
try {
// In test environments, visibleTextEditors might be empty, so also check if document exists
const isInVisibleEditors = vscode.window.visibleTextEditors.includes(this.editor)
const hasValidDocument = this.editor.document && !this.editor.document.isClosed
// If we're in a test environment (no visible editors), rely on document validity
// Otherwise, check both conditions
if (vscode.window.visibleTextEditors.length === 0) {
return hasValidDocument
}
return isInVisibleEditors && hasValidDocument
} catch {
// If accessing editor properties throws, it's disposed
return false
}
}
}

View file

@ -119,7 +119,7 @@ export class DiffViewProvider {
const diffEditor = this.activeDiffEditor
const document = diffEditor?.document
if (!diffEditor || !document) {
if (!diffEditor || !document || !this.isEditorValid(diffEditor)) {
throw new Error("User closed text editor, unable to edit file...")
}
@ -181,7 +181,10 @@ export class DiffViewProvider {
}
}
async saveChanges(diagnosticsEnabled: boolean = true, writeDelayMs: number = DEFAULT_WRITE_DELAY_MS): Promise<{
async saveChanges(
diagnosticsEnabled: boolean = true,
writeDelayMs: number = DEFAULT_WRITE_DELAY_MS,
): Promise<{
newProblemsMessage: string | undefined
userEdits: string | undefined
finalContent: string | undefined
@ -216,22 +219,22 @@ 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.
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(
@ -549,7 +552,7 @@ export class DiffViewProvider {
}
private scrollEditorToLine(line: number) {
if (this.activeDiffEditor) {
if (this.activeDiffEditor && this.isEditorValid(this.activeDiffEditor)) {
const scrollLine = line + 4
this.activeDiffEditor.revealRange(
@ -560,7 +563,7 @@ export class DiffViewProvider {
}
scrollToFirstDiff() {
if (!this.activeDiffEditor) {
if (!this.activeDiffEditor || !this.isEditorValid(this.activeDiffEditor)) {
return
}
@ -599,6 +602,14 @@ export class DiffViewProvider {
}
async reset(): Promise<void> {
// Dispose decoration controllers before clearing references
if (this.fadedOverlayController) {
this.fadedOverlayController.dispose()
}
if (this.activeLineController) {
this.activeLineController.dispose()
}
await this.closeAllDiffViews()
this.editType = undefined
this.isEditing = false
@ -611,4 +622,25 @@ export class DiffViewProvider {
this.streamedLines = []
this.preDiagnostics = []
}
private isEditorValid(editor: vscode.TextEditor): boolean {
// Check if the editor is still valid by verifying it exists in visible editors
// and its document hasn't been closed
try {
// In test environments, visibleTextEditors might be empty, so also check if document exists
const isInVisibleEditors = vscode.window.visibleTextEditors.includes(editor)
const hasValidDocument = editor.document && !editor.document.isClosed
// If we're in a test environment (no visible editors), rely on document validity
// Otherwise, check both conditions
if (vscode.window.visibleTextEditors.length === 0) {
return hasValidDocument
}
return isInVisibleEditors && hasValidDocument
} catch {
// If accessing editor properties throws, it's disposed
return false
}
}
}