Skip to content

Conversation

@daniel-lxs
Copy link
Member

@daniel-lxs daniel-lxs commented Jun 21, 2025

Description

Fixes #2360

This PR resolves an issue where file editing tools (apply_diff, write_to_file, search_and_replace) fail for files that have custom editor associations configured in VS Code, particularly markdown files with preview mode associations.

Root Cause

When VS Code has workbench.editorAssociations configured to open certain file types (like "*.md": "vscode.markdown.preview.editor"), the vscode.diff command opens the file in preview mode instead of as an editable text document. This causes:

  1. "Illegal value for lineNumber" errors when trying to edit
  2. Diff views that can't be properly detected and closed

Solution

Implemented a simple and reliable approach:

  1. Pre-open files as text documents: Before executing the diff command, always call vscode.window.showTextDocument() with preview: false to ensure the file opens in text editing mode
  2. Enhanced diff view detection: Updated closeAllDiffViews() to identify diff tabs both by URI scheme and by label pattern ("↔ Roo's Changes")

This PR doesn't affect the normal behavior of editing files at all. Files were being opened when edited, but this happened after the edit itself. Now it happens before the edit. Ultimately, the result is exactly the same.

Changes Made

  • src/integrations/editor/DiffViewProvider.ts:

    • Modified openDiffEditor() to pre-open files as text documents before executing diff command
    • Enhanced closeAllDiffViews() to detect diff views by label pattern as fallback
    • Added comprehensive comments explaining the fix
  • src/integrations/editor/__tests__/DiffViewProvider.spec.ts:

    • Updated mocks to include all required VSCode APIs
    • Added tests verifying the pre-opening behavior
    • Added tests for enhanced diff view closing logic
    • All existing tests continue to pass

Testing

  • All existing tests pass
  • Added new tests for pre-opening behavior
  • Added tests for enhanced diff view detection
  • Manual testing with markdown files and preview associations
  • Verified no performance regression

Verification of Acceptance Criteria

  • File editing tools work correctly with markdown files that have preview associations
  • Diff views are properly opened and closed
  • No "Illegal value for lineNumber" errors
  • Solution works for all file types, not just markdown
  • Backward compatibility maintained

Impact

  • Positive: Fixes a blocking issue for users with custom editor associations
  • Performance: Minimal impact - adds one additional showTextDocument call per diff operation
  • Compatibility: Fully backward compatible, no breaking changes
  • Scope: Affects only the diff editor functionality

Screenshots/Demo

Before: File editing tools would fail with "Illegal value for lineNumber" errors for markdown files with preview associations.

After: File editing tools work seamlessly regardless of editor associations.

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • Comments added for complex logic
  • All tests pass
  • No breaking changes
  • Issue requirements fully addressed

Important

Fixes diff editor issues in VS Code by pre-opening files as text documents and enhancing diff view detection.

  • Behavior:
    • Fixes issue with vscode.diff command opening files in preview mode due to workbench.editorAssociations.
    • Pre-opens files as text documents using vscode.window.showTextDocument() with preview: false.
    • Enhances closeAllDiffViews() in DiffViewProvider.ts to detect diff views by URI scheme and label pattern.
  • Testing:
    • Updates DiffViewProvider.spec.ts to mock necessary VSCode APIs.
    • Adds tests for pre-opening behavior and enhanced diff view detection.
    • Verifies no performance regression and maintains backward compatibility.

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

…4946)

- Pre-open files as text documents before executing diff command to avoid preview mode
- Update closeAllDiffViews to identify diff tabs by label pattern
- Add comprehensive tests for the new behavior

This ensures that files with custom editor associations (like markdown preview)
can be properly edited using Roo Code's diff editor tools.
@daniel-lxs daniel-lxs requested review from cte, jr and mrubens as code owners June 21, 2025 15:49
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Jun 21, 2025
@daniel-lxs daniel-lxs moved this from Triage to PR [Needs Review] in Roo Code Roadmap Jun 21, 2025
Copy link
Collaborator

@mrubens mrubens left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good other than the comment about using a constant for Original ↔ Roo's Changes and looking for that

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Jun 21, 2025
- Add DIFF_VIEW_LABEL_SEPARATOR constant to prevent breaking logic if string changes
- Update all usages in DiffViewProvider.ts and test file
- Addresses PR review feedback for better maintainability
- Rename DIFF_VIEW_LABEL_SEPARATOR to DIFF_VIEW_LABEL_CHANGES
- Include full 'Original ↔ Roo's Changes' text in constant for better maintainability
- Update all usages in DiffViewProvider.ts and test file
- Addresses feedback to make constant changes less awkward
@mrubens mrubens merged commit 8eb1473 into main Jun 22, 2025
10 checks passed
@mrubens mrubens deleted the fix/issue-4946-markdown-preview-diff-editor branch June 22, 2025 21:44
@github-project-automation github-project-automation bot moved this from PR [Needs Review] to Done in Roo Code Roadmap Jun 22, 2025
@github-project-automation github-project-automation bot moved this from New to Done in Roo Code Roadmap Jun 22, 2025
cte pushed a commit that referenced this pull request Jun 24, 2025
…4946) (#4980)

* fix: resolve diff editor issues with markdown preview associations (#4946)

- Pre-open files as text documents before executing diff command to avoid preview mode
- Update closeAllDiffViews to identify diff tabs by label pattern
- Add comprehensive tests for the new behavior

This ensures that files with custom editor associations (like markdown preview)
can be properly edited using Roo Code's diff editor tools.

* refactor: extract diff view label separator as shared constant

- Add DIFF_VIEW_LABEL_SEPARATOR constant to prevent breaking logic if string changes
- Update all usages in DiffViewProvider.ts and test file
- Addresses PR review feedback for better maintainability

* refactor: improve diff view label constant to include full text

- Rename DIFF_VIEW_LABEL_SEPARATOR to DIFF_VIEW_LABEL_CHANGES
- Include full 'Original ↔ Roo's Changes' text in constant for better maintainability
- Update all usages in DiffViewProvider.ts and test file
- Addresses feedback to make constant changes less awkward
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer PR - Needs Review size:L This PR changes 100-499 lines, ignoring generated files.

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Automatic detection and handling of Markdown Preview conflict when using write_to_file tool

4 participants