Skip to content

fix: handle consecutive added empty file diff #38

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 18, 2025

Conversation

yeonjuan
Copy link
Owner

fixes #31

@yeonjuan yeonjuan requested a review from Copilot May 18, 2025 16:28
Copy link

@Copilot 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 handling of consecutive empty-file diffs and refines parsing logic to correctly classify file changes.

  • Update comparison-line parsing to streamline to/from extraction
  • Change extended-header handling to skip comparison lines without consuming them
  • Add fixtures, tests, and snapshots for empty-file and no-prefix scenarios; update existing snapshots

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/parse-git-diff.ts Adjust token parsing in parseComparisonInputLine and header flow in parseExtendedHeader
src/tests/consecutive-empty-files.test.ts Add test for parsing consecutive empty-file diffs
src/tests/snapshots/consecutive-empty-files.test.ts.snap New snapshot for empty-file test
src/tests/snapshots/31.test.ts.snap Update snapshot to reflect new file type classification
src/tests/31.test.ts Remove .only from existing test suite
src/tests/31-no-prefix.test.ts Add test for noPrefix option
src/fixtures/consecutive-empty-files Add fixture representing multiple empty-file diffs
src/fixtures/31-no-prefix Add fixture for no-prefix diff scenario
Comments suppressed due to low confidence (1)

src/parse-git-diff.ts:138

  • Destructuring the reversed split tokens can misassign 'to' and 'from' if the diff line contains extra segments. Consider filtering parts by 'b/' and 'a/' prefixes (e.g., using find) to ensure correct file path extraction.
const [to, from] = line.split(' ').reverse();

@@ -210,7 +207,7 @@ function parseChunk(context: Context): AnyChunk | undefined {

function parseExtendedHeader(ctx: Context) {
if (isComparisonInputLine(ctx.getCurLine())) {
Copy link
Preview

Copilot AI May 18, 2025

Choose a reason for hiding this comment

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

Returning null without consuming the comparison line may leave the parser cursor on the same line and disrupt subsequent parsing. Consider calling ctx.nextLine() before returning to advance properly.

Suggested change
if (isComparisonInputLine(ctx.getCurLine())) {
if (isComparisonInputLine(ctx.getCurLine())) {
ctx.nextLine();

Copilot uses AI. Check for mistakes.

@yeonjuan yeonjuan merged commit 4ebd5f9 into main May 18, 2025
3 checks passed
@yeonjuan yeonjuan deleted the handle-consecutive-empty-files branch May 18, 2025 16:35
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.

Diff not parsing for added file
1 participant