Skip to content

#4199 split screen diff error fix#4385

Closed
Mnehmos wants to merge 1 commit intoRooCodeInc:mainfrom
Mnehmos:#4199-split-screen-diff-error
Closed

#4199 split screen diff error fix#4385
Mnehmos wants to merge 1 commit intoRooCodeInc:mainfrom
Mnehmos:#4199-split-screen-diff-error

Conversation

@Mnehmos
Copy link
Contributor

@Mnehmos Mnehmos commented Jun 5, 2025

Related GitHub Issue

Closes: #4199

Description

This PR addresses GitHub Issue #4199, where Roo Code failed to reliably detect the activation of the diff editor when its webview panel was moved to an auxiliary VS Code window (e.g., on a separate monitor). This resulted in a timeout and the error "Failed to open diff editor, please try again...".

The primary fix involves a more robust detection mechanism within src/integrations/editor/DiffViewProvider.ts:

  • Direct Tab Group Inspection: After the vscode.diff command is executed, the code now immediately iterates through all vscode.window.tabGroups.all to find the newly opened diff editor. This allows for detection across all VS Code windows, including auxiliary ones.
  • Enhanced Event Listening: A listener for vscode.window.tabGroups.onDidChangeTabGroups has been added. This event is triggered when tabs are moved between windows or new windows are opened, providing a more reliable signal for detecting the diff editor's appearance in multi-window setups.
  • Consolidated Resolution: Once the diff editor tab is identified (either through direct inspection or the event listener), vscode.window.showTextDocument is used to obtain the TextEditor instance and resolve the promise, confirming activation.
  • Improved Timeout Management: The detection timeout is now correctly cleared upon successful detection to prevent premature errors.

A subsequent commit in this PR also addresses an ESLint warning (no-async-promise-executor) that was present in src/integrations/editor/DiffViewProvider.ts. The Promise executor was refactored using an IIFE (Immediately Invoked Function Expression) to comply with the linting rule while preserving the asynchronous logic required for diff detection.

Test Procedure

Manual Verification (Essential for the core bug fix):

  1. Open Roo Code in an editor window within VS Code.
  2. Move the Roo editor window to a secondary monitor. This should cause it to operate as an auxiliary VS Code window.
  3. Attempt a file operation that invokes the diff editor (e.g., using apply_diff on an existing file, or write_to_file if it would trigger a diff for an existing file).
  4. Expected Result: The diff editor should open in the main VS Code window, and Roo Code should detect its activation without timing out or displaying the "Failed to open diff editor, please try again..." error. The operation should proceed as normal after the diff editor is closed/saved.
  5. Also, test the same operations with the Roo Code panel remaining in the primary VS Code window to ensure no regressions in the single-monitor setup.

Linting Verification:

  • Run pnpm lint (or the project's equivalent linting command) to confirm that the no-async-promise-executor warning in src/integrations/editor/DiffViewProvider.ts is resolved. This was confirmed during development.

Automated tests for the specific multi-monitor, auxiliary window interaction are complex to implement and are not included in this PR.

Type of Change

  • 🐛 Bug Fix: Non-breaking change that fixes an issue.
  • New Feature: Non-breaking change that adds functionality.
  • 💥 Breaking Change: Fix or feature that would cause existing functionality to not work as expected.
  • ♻️ Refactor: Code change that neither fixes a bug nor adds a feature.
  • 💅 Style: Changes that do not affect the meaning of the code (white-space, formatting, etc.).
  • 📚 Documentation: Updates to documentation files.
  • ⚙️ Build/CI: Changes to the build process or CI configuration.
  • 🧹 Chore: Other changes that don't modify src or test files.

Pre-Submission Checklist

  • Issue Linked: This PR is linked to an approved GitHub Issue (see "Related GitHub Issue" above).
  • Scope: My changes are focused on the linked issue (one major feature/fix per PR).
  • Self-Review: I have performed a thorough self-review of my code.
  • Code Quality:
    • My code adheres to the project's style guidelines.
    • There are no new linting errors or warnings (npm run lint).
    • All debug code (e.g., console.log) used during development of this fix has been reviewed and necessary diagnostic logs are retained as per the scope.
  • Testing:
    • New and/or updated tests have been added to cover my changes. (Manual testing is primary for the core bug due to environmental complexity for automation).
    • All tests pass locally (npm test). (Linting passes; core functionality requires manual verification as outlined).
    • The application builds successfully with my changes.
  • Branch Hygiene: My branch is up-to-date (rebased) with the main branch. (User should ensure this before merging).
  • Documentation Impact: I have considered if my changes require documentation updates (see "Documentation Updates" section below).
  • Changeset: A changeset has been created using npm run changeset if this PR includes user-facing changes or dependency updates. (This is a user-facing bug fix, so a changeset is recommended).
  • Contribution Guidelines: I have read and agree to the Contributor Guidelines.

Screenshots / Videos

N/A (This is a functional fix related to editor event detection, not a UI change).

Documentation Updates

  • No documentation updates are required.

Additional Notes

The fix was implemented in two stages:

  1. Addressing the core diff editor detection logic.
  2. Resolving a subsequent ESLint warning introduced by the initial fix.

image

Get in Touch


Important

Fixes diff editor detection in DiffViewProvider for multi-window setups and resolves ESLint warning.

  • Behavior:
    • Fixes detection of diff editor activation in DiffViewProvider when moved to auxiliary VS Code windows.
    • Uses direct tab group inspection and enhanced event listening for reliable detection.
    • Clears detection timeout upon successful detection to prevent errors.
  • Code Quality:
    • Refactors Promise executor in DiffViewProvider using an IIFE to resolve no-async-promise-executor ESLint warning.

This description was created by Ellipsis for 411d95e. You can customize this summary. It will automatically update as commits are pushed.

@Mnehmos Mnehmos requested review from cte and mrubens as code owners June 5, 2025 18:01
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Jun 5, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Prelim Review] in Roo Code Roadmap Jun 5, 2025
@Mnehmos Mnehmos closed this by deleting the head repository Jun 5, 2025
@daniel-lxs daniel-lxs moved this from PR [Needs Prelim Review] to Done in Roo Code Roadmap Jun 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working size:L This PR changes 100-499 lines, ignoring generated files.

Projects

No open projects
Archived in project

Development

Successfully merging this pull request may close these issues.

Roo Code fails to open diff editor in specific VS Code contexts

1 participant

Comments