Skip to content

Commit 45249e1

Browse files
felixAnhaltfelixAnhalt
committed
refactor(diff): enhance tab closing logic and settings management in DiffViewProvider
1 parent 542c7fb commit 45249e1

File tree

1 file changed

+108
-127
lines changed

1 file changed

+108
-127
lines changed

‎src/integrations/editor/DiffViewProvider.ts‎

Lines changed: 108 additions & 127 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,12 @@ import { ClineProvider } from "../../core/webview/ClineProvider"
1515

1616
export const DIFF_VIEW_URI_SCHEME = "cline-diff"
1717

18+
interface DiffSettings {
19+
autoFocus: boolean
20+
autoCloseRooTabs: boolean
21+
autoCloseAllRooTabs: boolean
22+
}
23+
1824
// TODO: https://github.com/cline/cline/pull/3354
1925
export class DiffViewProvider {
2026
editType?: "create" | "modify"
@@ -62,7 +68,7 @@ export class DiffViewProvider {
6268
this.rooOpenedTabs.clear()
6369
}
6470

65-
private async _readDiffSettings() {
71+
private async _readDiffSettings(): Promise<DiffSettings> {
6672
const config = vscode.workspace.getConfiguration("roo-cline")
6773
const autoFocus = config.get<boolean>("diffViewAutoFocus", true)
6874
const autoCloseRooTabs = config.get<boolean>("autoCloseRooTabs", false)
@@ -500,151 +506,126 @@ export class DiffViewProvider {
500506
this.resetWithListeners()
501507
}
502508

503-
private async closeAllRooOpenedViews() {
504-
const settings = await this._readDiffSettings() // Dynamically read settings
505-
506-
const tabs = vscode.window.tabGroups.all
507-
.flatMap((tg) => tg.tabs)
508-
.filter((tab) => {
509-
// Always close DiffView tabs opened by Roo
510-
if (
511-
tab.input instanceof vscode.TabInputTextDiff &&
512-
tab.input?.original?.scheme === DIFF_VIEW_URI_SCHEME
513-
) {
514-
return true
515-
}
509+
private filterTabsToClose(tab: vscode.Tab, settings: DiffSettings): boolean {
510+
// Always close DiffView tabs opened by Roo
511+
if (tab.input instanceof vscode.TabInputTextDiff && tab.input?.original?.scheme === DIFF_VIEW_URI_SCHEME) {
512+
return true
513+
}
516514

517-
let isRooOpenedTextTab = false
518-
if (tab.input instanceof vscode.TabInputText) {
519-
const currentTabUri = (tab.input as vscode.TabInputText).uri
520-
for (const openedUriString of this.rooOpenedTabs) {
521-
try {
522-
const previouslyOpenedUri = vscode.Uri.parse(openedUriString, true) // true for strict parsing
523-
if (currentTabUri.scheme === "file" && previouslyOpenedUri.scheme === "file") {
524-
if (arePathsEqual(currentTabUri.fsPath, previouslyOpenedUri.fsPath)) {
525-
isRooOpenedTextTab = true
526-
break
527-
}
528-
} else {
529-
if (currentTabUri.toString() === previouslyOpenedUri.toString()) {
530-
isRooOpenedTextTab = true
531-
break
532-
}
533-
}
534-
} catch (e) {
535-
// Log parsing error if necessary, or ignore if a URI in rooOpenedTabs is malformed
536-
console.error(`Roo Debug: Error parsing URI from rooOpenedTabs: ${openedUriString}`, e)
515+
let isRooOpenedTextTab = false
516+
if (tab.input instanceof vscode.TabInputText) {
517+
const currentTabUri = (tab.input as vscode.TabInputText).uri
518+
for (const openedUriString of this.rooOpenedTabs) {
519+
try {
520+
const previouslyOpenedUri = vscode.Uri.parse(openedUriString, true) // true for strict parsing
521+
if (currentTabUri.scheme === "file" && previouslyOpenedUri.scheme === "file") {
522+
if (arePathsEqual(currentTabUri.fsPath, previouslyOpenedUri.fsPath)) {
523+
isRooOpenedTextTab = true
524+
break
525+
}
526+
} else {
527+
if (currentTabUri.toString() === previouslyOpenedUri.toString()) {
528+
isRooOpenedTextTab = true
529+
break
537530
}
538531
}
532+
} catch (e) {
533+
// Log parsing error if necessary, or ignore if a URI in rooOpenedTabs is malformed
534+
console.error(`Roo Debug: Error parsing URI from rooOpenedTabs: ${openedUriString}`, e)
539535
}
536+
}
537+
}
540538

541-
if (!isRooOpenedTextTab) {
542-
return false // Not a text tab or not identified as opened by Roo
543-
}
539+
if (!isRooOpenedTextTab) {
540+
return false // Not a text tab or not identified as opened by Roo
541+
}
544542

545-
// Haken 2 (settings.autoCloseAllRooTabs) - takes precedence
546-
if (settings.autoCloseAllRooTabs) {
547-
// This implies Haken 1 is also effectively on
548-
return true // Close all Roo-opened text tabs
549-
}
543+
// Haken 2 (settings.autoCloseAllRooTabs) - takes precedence
544+
if (settings.autoCloseAllRooTabs) {
545+
// This implies Haken 1 is also effectively on
546+
return true // Close all Roo-opened text tabs
547+
}
550548

551-
// Only Haken 1 (settings.autoCloseRooTabs) is on, Haken 2 is off
552-
if (settings.autoCloseRooTabs) {
553-
const tabUriFsPath = (tab.input as vscode.TabInputText).uri.fsPath
554-
const absolutePathDiffedFile = this.relPath ? path.resolve(this.cwd, this.relPath) : null
549+
// Only Haken 1 (settings.autoCloseRooTabs) is on, Haken 2 is off
550+
if (settings.autoCloseRooTabs) {
551+
const tabUriFsPath = (tab.input as vscode.TabInputText).uri.fsPath
552+
const absolutePathDiffedFile = this.relPath ? path.resolve(this.cwd, this.relPath) : null
555553

556-
// Guard against null absolutePathDiffedFile if relPath is somehow not set
557-
if (!absolutePathDiffedFile) {
558-
// If we don't know the main diffed file, but Haken 1 is on,
559-
// it's safer to close any tab Roo opened to avoid leaving extras.
560-
return true
561-
}
554+
// Guard against null absolutePathDiffedFile if relPath is somehow not set
555+
if (!absolutePathDiffedFile) {
556+
// If we don't know the main diffed file, but Haken 1 is on,
557+
// it's safer to close any tab Roo opened to avoid leaving extras.
558+
return true
559+
}
562560

563-
const isMainDiffedFileTab = arePathsEqual(tabUriFsPath, absolutePathDiffedFile)
561+
const isMainDiffedFileTab = arePathsEqual(tabUriFsPath, absolutePathDiffedFile)
564562

565-
if (this.editType === "create" && isMainDiffedFileTab) {
566-
return true // Case: New file, Haken 1 is on -> Close its tab.
567-
}
563+
if (this.editType === "create" && isMainDiffedFileTab) {
564+
return true // Case: New file, Haken 1 is on -> Close its tab.
565+
}
568566

569-
if (this.editType === "modify" && isMainDiffedFileTab) {
570-
return !this.documentWasOpen
571-
}
567+
if (this.editType === "modify" && isMainDiffedFileTab) {
568+
return !this.documentWasOpen
569+
}
572570

573-
// If the tab is for a file OTHER than the main diffedFile, but was opened by Roo
574-
if (!isMainDiffedFileTab) {
575-
// This covers scenarios where Roo might open auxiliary files (though less common for single diff).
576-
// If Haken 1 is on, these should also be closed.
577-
return true
578-
}
579-
}
580-
return false // Default: do not close if no above condition met
581-
})
571+
// If the tab is for a file OTHER than the main diffedFile, but was opened by Roo
572+
if (!isMainDiffedFileTab) {
573+
// This covers scenarios where Roo might open auxiliary files (though less common for single diff).
574+
// If Haken 1 is on, these should also be closed.
575+
return true
576+
}
577+
}
578+
return false // Default: do not close if no above condition met
579+
}
582580

583-
for (const tab of tabs) {
584-
// If a tab has made it through the filter, it means one of the auto-close settings
585-
// (autoCloseTabs or autoCloseAllRooTabs) is active and the conditions for closing
586-
// this specific tab are met. Therefore, we should always bypass the dirty check.
587-
// const bypassDirtyCheck = true; // This is implicitly true now.
581+
private async closeTab(tab: vscode.Tab) {
582+
// If a tab has made it through the filter, it means one of the auto-close settings
583+
// (autoCloseTabs or autoCloseAllRooTabs) is active and the conditions for closing
584+
// this specific tab are met. Therefore, we should always bypass the dirty check.
585+
// const bypassDirtyCheck = true; // This is implicitly true now.
588586

589-
// Attempt to find the freshest reference to the tab before closing,
590-
// as the original 'tab' object from the initial flatMap might be stale.
591-
const tabInputToClose = tab.input
592-
const freshTabToClose = vscode.window.tabGroups.all
593-
.flatMap((group) => group.tabs)
594-
.find((t) => t.input === tabInputToClose)
587+
// Attempt to find the freshest reference to the tab before closing,
588+
// as the original 'tab' object from the initial flatMap might be stale.
589+
const tabInputToClose = tab.input
590+
const freshTabToClose = vscode.window.tabGroups.all
591+
.flatMap((group) => group.tabs)
592+
.find((t) => t.input === tabInputToClose)
595593

596-
if (freshTabToClose) {
597-
try {
598-
await vscode.window.tabGroups.close(freshTabToClose, true) // true to bypass dirty check implicitly
599-
} catch (closeError) {
600-
console.error(`Roo Debug CloseLoop: Error closing tab "${freshTabToClose.label}":`, closeError)
601-
}
602-
} else {
603-
// This case should ideally not happen if the tab was in the filtered list.
604-
// It might indicate the tab was closed by another means or its input changed.
605-
console.warn(
606-
`Roo Debug CloseLoop: Tab "${tab.label}" (input: ${JSON.stringify(tab.input)}) intended for closure was not found in the current tab list.`,
594+
if (freshTabToClose) {
595+
try {
596+
await vscode.window.tabGroups.close(freshTabToClose, true) // true to bypass dirty check implicitly
597+
} catch (closeError) {
598+
console.error(`Roo Debug CloseLoop: Error closing tab "${freshTabToClose.label}":`, closeError)
599+
}
600+
} else {
601+
// This case should ideally not happen if the tab was in the filtered list.
602+
// It might indicate the tab was closed by another means or its input changed.
603+
console.warn(
604+
`Roo Debug CloseLoop: Tab "${tab.label}" (input: ${JSON.stringify(tab.input)}) intended for closure was not found in the current tab list.`,
605+
)
606+
// Fallback: Try to close the original tab reference if the fresh one isn't found,
607+
// though this is less likely to succeed if it's genuinely stale.
608+
try {
609+
console.log(`Roo Debug CloseLoop: Attempting to close original (stale?) tab "${tab.label}"`)
610+
await vscode.window.tabGroups.close(tab, true)
611+
} catch (fallbackCloseError) {
612+
console.error(
613+
`Roo Debug CloseLoop: Error closing original tab reference for "${tab.label}":`,
614+
fallbackCloseError,
607615
)
608-
// Fallback: Try to close the original tab reference if the fresh one isn't found,
609-
// though this is less likely to succeed if it's genuinely stale.
610-
try {
611-
console.log(`Roo Debug CloseLoop: Attempting to close original (stale?) tab "${tab.label}"`)
612-
await vscode.window.tabGroups.close(tab, true)
613-
} catch (fallbackCloseError) {
614-
console.error(
615-
`Roo Debug CloseLoop: Error closing original tab reference for "${tab.label}":`,
616-
fallbackCloseError,
617-
)
618-
}
619616
}
620617
}
621618
}
622619

623-
private async getEditorFromDiffTab(uri: vscode.Uri): Promise<vscode.TextEditor | null> {
624-
// If this diff editor is already open (ie if a previous write file was interrupted) then we should activate that instead of opening a new diff
625-
const diffTab = vscode.window.tabGroups.all
626-
.flatMap((group) => group.tabs)
627-
.find(
628-
(tab) =>
629-
tab.input instanceof vscode.TabInputTextDiff &&
630-
tab.input?.original?.scheme === DIFF_VIEW_URI_SCHEME &&
631-
arePathsEqual(tab.input.modified.fsPath, uri.fsPath),
632-
)
633-
// If this diff editor is already open (ie if a previous write file was
634-
// interrupted) then we should activate that instead of opening a new
635-
// diff.
636-
if (!(diffTab && diffTab.input instanceof vscode.TabInputTextDiff)) {
637-
return null
638-
}
639-
// Removed logic that explicitly opens/focuses the standalone file tab immediately after diff opens.
640-
// The standalone tab should only be opened in saveChanges after approval for new/unopened files.
641-
const editor = vscode.window.visibleTextEditors.find((ed) => arePathsEqual(ed.document.uri.fsPath, uri.fsPath))
642-
if (editor) return editor
643-
644-
// If the editor is not found among visible editors, it means it's not currently open.
645-
// We should not open it here, as it should only be opened in saveChanges after approval.
646-
// Return null to indicate that the editor for the standalone file tab is not available/should not be focused immediately.
647-
return null
620+
private async closeAllRooOpenedViews() {
621+
const settings = await this._readDiffSettings() // Dynamically read settings
622+
623+
const closeOps = vscode.window.tabGroups.all
624+
.flatMap((tg) => tg.tabs)
625+
.filter((tab) => this.filterTabsToClose(tab, settings))
626+
.map(this.closeTab)
627+
628+
await Promise.all(closeOps)
648629
}
649630

650631
/**
@@ -671,7 +652,7 @@ export class DiffViewProvider {
671652
const title = `${fileName}: ${fileExists ? "Original ↔ Roo's Changes" : "New File"} (Editable)`
672653
const textDocumentShowOptions: TextDocumentShowOptions = {
673654
preview: false,
674-
preserveFocus: settings.autoFocus ? false : true, // Use dynamically read autoFocus
655+
preserveFocus: !settings.autoFocus, // Use dynamically read autoFocus
675656
viewColumn: this.viewColumn,
676657
}
677658
// set interaction flag to true to prevent autoFocus from being triggered

0 commit comments

Comments
 (0)