Skip to content

diff: correct diff stat alignment in presence of renames w/ common prefix. #7057

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 3 commits into from
Jun 5, 2025

Conversation

kivikakk
Copy link
Contributor

Hi old friends!

I noticed that libgit2's diff stat has its alignment get thrown out in the presence of renames where there's a common prefix, e.g.:

 lib/nos_web/controllers/user_controller.ex                                   | 7 ++++++-
 lib/nos_web/controllers/{page_html.ex => user_html.ex} | 8 ++-----
 lib/nos_web/controllers/user_html/show.html.heex                             | 3 +++-

This PR adds a test for this (fails on main), and then fixes it by reproducing the common prefix detection when calculating git_diff_stats's max_name member. That now fully accounts for the various formats (i.e. samename, oldname => newname, prefix{oldsuffix => newsuffix}).

This removed the sole use of renames in the same struct, so I dropped it.

@ethomson
Copy link
Member

ethomson commented Jun 5, 2025

Thanks for the fix!

@ethomson ethomson merged commit d0da681 into libgit2:main Jun 5, 2025
14 checks passed
@kivikakk
Copy link
Contributor Author

kivikakk commented Jun 5, 2025

Thanks! :)

@kivikakk kivikakk deleted the diff-stat-alignment branch June 5, 2025 13:02
@ethomson ethomson added the bug label Jun 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants