Skip to content

fix(diff): honor renamesFromRewrites in findSimilar - #236

Open
poketopa wants to merge 2 commits into
toss:mainfrom
poketopa:fix/diff-renames-from-rewrites
Open

poketopa wants to merge 2 commits into
toss:mainfrom
poketopa:fix/diff-renames-from-rewrites

Conversation

@poketopa

Copy link
Copy Markdown
Contributor

Summary

Diff#findSimilar passed renamesFromRewrites to git2::DiffFindOptions::renames(), so the option toggled GIT_DIFF_FIND_RENAMES instead of GIT_DIFF_FIND_RENAMES_FROM_REWRITES:

  • renamesFromRewrites: true never used the old side of a rewritten file as a rename source. When A is rewritten and its old content moves to A-renamed, the result was Modified A + Added A-renamed instead of Added A + Renamed A -> A-renamed.
  • renamesFromRewrites: false cleared an explicit renames: true. With no find flag left, libgit2 falls back to diff.renames, so renames were not detected when that config is false.

Changes

  • Forward renamesFromRewrites to renames_from_rewrites(), the setter StatusOptions already uses for its field of the same name.
  • Add one case per symptom to tests/diff.spec.ts. Both fail on main and pass with this change.

The public API and the generated index.js / index.d.ts are unchanged. libgit2 turns on GIT_DIFF_FIND_RENAMES whenever GIT_DIFF_FIND_RENAMES_FROM_REWRITES is set, so callers passing only renamesFromRewrites: true still get rename detection. They now also get rewritten files used as rename sources, which is what the option documents.

Validation

Verified locally on macOS ARM64 with Rust 1.91.0 and Node.js 24.1.0:

  • yarn install --immutable
  • just format, just lint, and just typecheck
  • just test: Vitest reported 244 passed and 1 existing test skipped; the Rust test target has no tests
  • tests/diff.spec.ts against main: both new cases fail (Modified + Added, and Deleted + Added)
  • Fresh yarn build matches the committed index.js and index.d.ts
  • git diff --check

Add two cases for Diff#findSimilar:

- With renamesFromRewrites, the old side of a rewritten file is a
  rename source, so moving its content to a new path is reported as
  Added + Renamed.
- renamesFromRewrites: false must not clear renames.

Both fail on main, where renamesFromRewrites is passed to renames().
DiffFindOptions.renamesFromRewrites was forwarded to
git2::DiffFindOptions::renames(), so it toggled GIT_DIFF_FIND_RENAMES
instead of GIT_DIFF_FIND_RENAMES_FROM_REWRITES. Setting it to true
never used rewritten files as rename sources, and setting it to false
cleared an explicit renames: true.

Forward it to renames_from_rewrites(), as StatusOptions already does.
@vercel

vercel Bot commented Oct 19, 2026

Copy link
Copy Markdown

@poketopa is attempting to deploy a commit to the Toss Team on Vercel.

A member of the Team first needs to authorize it.

This branch has not been deployed

No deployments
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.

1 participant