Skip to content

Inline diffs for large files and low computation timeouts - #6005

Open
mkslanc wants to merge 9 commits into
ajaxorg:masterfrom
mkslanc:diff-timeout-fix
Open

Inline diffs for large files and low computation timeouts#6005
mkslanc wants to merge 9 commits into
ajaxorg:masterfrom
mkslanc:diff-timeout-fix

Conversation

@mkslanc

@mkslanc mkslanc commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Issue #, if available:

Description of changes:
The default provider now performs a fast Myers (or Dynamic Programming for combined lines less than 1700) line-level diff for the entire document and marks modified line chunks as pending character refinement. Character-level diffs are calculated only for chunks in or near the visible viewport, using dynamic programming for small chunks and Myers for larger ones.

The renderer also skips redundant inline markers when an inline change covers the complete line-diff range.

For Diff Providers without refine method this logic is completely skipped.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Pull Request Checklist:

Open kitchen-sink @ 8b448c7c61a3d3d1408d04f1da9fcf866eab61fb

Open kitchen-sink @ 25c4ec412a8176a5e09473c1cbc9932eeabf4250

Open kitchen-sink @ d63fd5f0f5b6600ee6b7b1c9d0517368f8c09fc1

Open kitchen-sink @ ef76825defa4b08ff7c161b1321482d5cadb7965

Open kitchen-sink @ 677c4da7a7548d25ff4504a779b4c410770e34b7

Open kitchen-sink @ ca24ada14903fbd0b8dcb61ad10a937d082ae629

@nightwing

Copy link
Copy Markdown
Member

I've added two small fixes for marker rendering, but the refining is still too unreliable, e.g. if i do not have the delimiter line in the added test, diff thinks the whole chunk at the start is added.

@codecov

codecov Bot commented Aug 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.92961% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.92%. Comparing base (054a3d6) to head (ef76825).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
src/ext/diff/providers/default.js 86.84% 10 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6005      +/-   ##
==========================================
- Coverage   93.96%   93.92%   -0.04%     
==========================================
  Files         647      647              
  Lines      140305   140672     +367     
  Branches    14739    14741       +2     
==========================================
+ Hits       131836   132126     +290     
- Misses       8469     8546      +77     
Flag Coverage Δ
unittests 93.92% <97.92%> (-0.04%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…es now uses Dynamic Programming algo instead of Myers
@mkslanc

mkslanc commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

I've added two small fixes for marker rendering, but the refining is still too unreliable, e.g. if i do not have the delimiter line in the added test, diff thinks the whole chunk at the start is added.

New commit should fix this -> I returned Dynamic Programming call for documents with combined lines less than 1700 (as it was in previous implementation); lazy visible-only inline refinement remains unchanged.
Renamed test to reflect actual implementation and also added test for really big documents (>1700 lines). Tested without delimiters -> now it correctly determines changes on first 100 lines of the same test

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants