gitoxide icon indicating copy to clipboard operation
gitoxide copied to clipboard

Update `gix-blame` to `imara-diff` 0.2

Open cruessler opened this issue 2 weeks ago • 2 comments

This is a quick conversion of gix-blame to use imara-diff 0.2 through gix-diff’s new blob-experimental feature. I haven’t run any benchmarks yet, just wanted to open the PR, so you can have a look yourself! The update made one test that previously was marked should_panic pass, so that’s already promising. :-)

cruessler avatar Dec 07 '25 17:12 cruessler

This looks very promising indeed! Some complexity now moved to imara-diff.

Let's see those performance comparisons as well :). Until we are more sure how this works, you could put this change behind a feature flag as well, supporting both implementations side-by-side. Alternatively, this can stay open until we know what to do with V2. And… once it's clear that it is not slower, I guess it can just be adopted, provided all existing unified diff tests can be ported which will be quite an undertaking I suppose.

Byron avatar Dec 07 '25 18:12 Byron

I added a feature flag, blob-experimental, to gix-blame. This should hopefully also make the full diff easier to read as it now mostly contains additions and very few changes.

I decided to re-use the comment to blob-experimental that I copied from gix-diff. Feel free to re-phrase, though.

I’ll mark this PR as ready even though CI hasn’t completed yet (so I can turn off my computer :-)). If anything comes up, I’ll address it, but apart from that, I hope it is finished.

The only thing that’s missing is proably a line such as the following one in justfile:

cargo nextest run -p gix-blame --features blob-experimental --no-fail-fast

cruessler avatar Dec 13 '25 14:12 cruessler

Thanks a lot, this looks very promising already! Now, technically, gix-blame is basically as good as the Git one in terms of correctness, and probably comes quite close in terms of performance. This makes me curious for performance comparisons as well, but my guess is that imara v1 and v2 have the same performance in practice.

Lastly, let me point out a critical issue that Copilot found which I 'fixed', but also noticed that there is no test that captures this particular handling. Now I wonder if Blame doesn't care due to its nature, so the special case could be dropped, or if we are really missing that one test that runs into this.

Byron avatar Dec 17 '25 06:12 Byron