Loading
Fix single file diff collapsed bug
What does this MR do and why?
Problem
- In a previous MR (!44629 (merged)), a functionality was added to automatically expand single diff files even when they exceed normal auto-expansion limits.
- However, when a MR contains a single large file diff, it may incorrectly appear collapsed due to inconsistent handling of diff counts between Gitaly and Array iterators. This especially happens when both
Show whitespace changesandShow one file at a timesettings are disabled.
Potential Root Cause
- This occurs because
@iterator.sizereturnsnilin the Gitaly case (due to streaming response), causing the expansion logic to fail, while it works correctly for Array iterators.
- With Gitaly's
DiffStitcher, a single diff can be split across multiple messages. The current@iterator.sizeapproach fails because:- It delegates to the gRPC streaming response's size, which returns
nil(size cannot be determined until the stream is consumed I believe) - Even if size was available, it would count messages rather than complete diffs (for example: one diff split into 3 messages would incorrectly trigger collapse when it should be expanded)
- It delegates to the gRPC streaming response's size, which returns
Solution
- Modify
DiffStitcherto track complete diffs internally by:- Adding a
@diff_countcounter inDiffStitcherthat increments only when a complete diff is assembled (end_of_patch) - Implementing a
sizemethod that returns the actual number of complete diffs - Removing delegation to the gRPC response's size method
- Adding a
An alternative approach would be to increment the diff counter in each_gitaly_patch and each_serialized_patch methods. However, encapsulating the counting logic within DiffStitcher provides a cleaner and more maintainable solution.
Testing
Note: make sure both Show whitespace changes and Show one file at a time settings are disabled
- Checkout to
masterbranch and create a MR with 1 large file. It should be collapsed - Checkout to
516100-fix-single-diff-file-collapsed-bugand create a MR with 1 large file. It shouldn't be collapsed - Checkout to
516100-fix-single-diff-file-collapsed-bugand create a MR with 1 large file and 1 normal file. Large file should be collapsed and normal file shouldn't be collapsed
Screenshots
| Before | After |
|---|---|
![]() |
![]() |
References
Please include cross links to any resources that are relevant to this MR. This will give reviewers and future readers helpful context to give an efficient review of the changes introduced.
- MR where this feature was first added: !44629 (merged)
- Issue: #516100 (closed)
MR acceptance checklist
Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.
Related to #516100 (closed)
Edited by Kinshuk Singh


