Skip to content
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

Count removed bytes correctly #1100

Merged
merged 1 commit into from
Nov 9, 2023
Merged

Count removed bytes correctly #1100

merged 1 commit into from
Nov 9, 2023

Conversation

caio
Copy link
Contributor

@caio caio commented Nov 9, 2023

Hello,

Thanks a lot for gix! I've been playing with it on and off for a while now and am very impressed with the speed and quality of your work.

One of the things I've done is implement my own version of git log -- somefile, which led me to doing my own rename detection and then my results started looking different from gix's

What follows is a patch to fix what I believe is the bug: removed_bytes is being overwritten every time the sink looks at a hunk of changes.

(Qualifying the sum() as sum::<usize>() is required due to multiple impls of std::ops::AddAssign on the type)

Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Thanks! And thanks for the fix as well, and even with a test, lovely!

I will merge when CI is green.

Regarding rename tracking, you might find interesting the gix-diff will soon receive a generalized version of the rename-tracking engine that already powers rename tracking of tree-diffs. If anything is missing for you that you think gix should support, please do let me know. In theory, it should be possible to use gix to perfectly re-implement git log for example, so the building blocks should be there. You can track this issue which probably will see more diff-related improvements - currently is still a few steps away from being correct in all cases.

@caio
Copy link
Contributor Author

caio commented Nov 9, 2023

Glad to hear it! I still have many small things I plan on using gix for- will let you know if anything comes up :)

@Byron Byron merged commit 7227410 into GitoxideLabs:main Nov 9, 2023
18 checks passed
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