Skip to content

Fix splitLineCount and unifiedLineCount on hunks#241

Merged
amadeus merged 8 commits intomainfrom
amadeus/parse-patch-fix
Dec 19, 2025
Merged

Fix splitLineCount and unifiedLineCount on hunks#241
amadeus merged 8 commits intomainfrom
amadeus/parse-patch-fix

Conversation

@amadeus
Copy link
Copy Markdown
Member

@amadeus amadeus commented Dec 18, 2025

A classic case of -- we didn't really depend on this so never totally realized it was wrong. Anyways, these values were wrong and it started showing up in my virtualization work because I depended on them.

Also added tests to make sure I don't fuck these up again

Comment thread packages/diffs/src/utils/parsePatchFiles.ts Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes incorrect calculations for splitLineCount and unifiedLineCount on diff hunks. The previous implementation incorrectly calculated these values as simple max/count of additions and deletions without accounting for context lines. The fix properly computes these counts by iterating through hunk content and summing context lines with the appropriate calculation for change blocks.

  • Updated the calculation logic to properly account for context lines in both split and unified modes
  • Added comprehensive tests that verify the calculated counts match actual rendered output
  • Updated all affected snapshots to reflect the corrected values

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

File Description
packages/diffs/src/utils/parsePatchFiles.ts Replaced simple max/count calculations with proper reduce logic that accounts for context lines in both splitLineCount and unifiedLineCount
packages/diffs/test/testUtils.ts Updated verification logic to match new calculation method and added helper functions to count rendered lines and split rows
packages/diffs/test/parsePatchFiles.test.ts Added two new tests that verify calculated counts match actual rendered output in both split and unified modes
packages/diffs/test/snapshots/*.snap Updated snapshot values to reflect corrected splitLineCount and unifiedLineCount calculations across all test files

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/diffs/test/parsePatchFiles.test.ts Outdated
Comment thread packages/diffs/test/parsePatchFiles.test.ts Outdated
Comment thread packages/diffs/test/parsePatchFiles.test.ts Outdated
amadeus and others added 6 commits December 18, 2025 15:38
* Lets be sure we are getting proper numbers out of `data-line-index`
* Why allocate massive array when 1 smol array do trick
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@amadeus amadeus merged commit 5119bd5 into main Dec 19, 2025
6 checks passed
@amadeus amadeus deleted the amadeus/parse-patch-fix branch December 19, 2025 00:17
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