Replace libgit2 with gitoxide

[blinxen]

As discussed in https://github.com/sharkdp/bat/discussions/3151, I have prepared a PR to replace the libgit2 usage in bat with gitoxide. The PR is not finished yet because some tests are failing since the produced git diff does not match the exact behavior of libgit2.

I see here two solutions:

• Extend the imara-diff crate to generate git like diffs
• Adapt the tests to accept the new diff format (it is not a format but more like a different diffing algorithm)

What do the maintainers think?

CC: @byron

[keith-hall]

Thanks for working on this! Honestly, it feels like the diffs are still valid, so adapting the tests is acceptable in my opinion. However you might find that extending the imara-diff crate to generate git-like diffs will make more people happy in the long run, so I would say that it depends what you feel up to really :slightly_smiling_face:

[Byron]

You may also find this document about diff-sliders interesting. It also details the tuning that Git has received to produce more 'likable' diffs, some of which libgit2 seems to inherit. It seems like an art as well.

Right now, imara-diff has received no time in the direction of optimizing diffs for humans, and I think it would have to be a community effort if that should happen.

As this comes up all the time, I also feel that pressure is mounting, and more so the more people use imara-diff :).

[blinxen]

I think that trying to improve imara-diff would be more interessting. I checked the git mailing list and found the initial patch that introduced what the diff-sliders document proposes. I will check it out and try to implement something similiar for imara-diff. At the mean time we can keep this PR open since it does not have any urgency.

[blinxen]

Since I have not been able to work much on the imara-diff improvements lately, I updated this PR so that it at least passes all tests.

[blinxen]

https://github.com/GitoxideLabs/gitoxide/pull/2011 just got merged and I tested the optimized diff locally and it works :D. I will updated this PR once a new version of gix is released. Then this PR will be open for review.

[Byron]

I am sorry for my negligence and wishful thinking when merging the implementation despite it having only one test, but it does indeed not appear to work correctly.

Having put it into GitButler to get some real-world experience with it, here we have an example where both Git and GitButler (with the standard hunk visualization) agree:

However, when using the new hunk processing to get a Git-like diff, this happens:

Of course it could be that the ChangeKind needs to be interpreted in some way, but even if so, I think it needs tests to make clear how this is working, the more the merrier.

For now I have removed the implementation from the codebase, but of course would be happy to see a follow-up PR that brings it back, along with tests, ideally even baseline tests where various expectations are produced by Git itself.

[blinxen]

That is not so good :grimacing:

I took a quick peek and something with the scoring is weird with this diff. For some reason the implementation prefers moving up to line 132 over staying at line 141. I will look into it and also try to come up with a good test suite.

[Byron]

Thank you, it's much appreciated! I could imagine that it's just one issue that's causing this and when fixed it will work flawlessly. But now we'd want to make sure and add every conceivable test to try and break it. Maybe one can bring in some tests from the diff-slider repo to get more coverage.

[blinxen]

Already found the issue --> https://github.com/blinxen/gitoxide/commit/d7ab68e659df3cd99bc69fd89932575a28f4691c

Maybe one can bring in some tests from the diff-slider repo to get more coverage.

That will be the next step

[keith-hall]

Any new updates on this one? :)

[blinxen]

I did not have enough time lately to continue to work on the diff improvements. However, the imara-diff maintainer implemented the same algorithm and merged it into Helix. That might be a better direction to take. Until is solved https://github.com/pascalkuthe/imara-diff/issues/27, we can't use it though.

Thanks for working on this! Honestly, it feels like the diffs are still valid, so adapting the tests is acceptable in my opinion.

If this statement is still valid then I can solve the merge conflicts and we can just merge it. Improvements can be added in a later PR.

[keith-hall]

I do worry that many users will complain that the diffs don't match what git shows... Perhaps having a feature flag to decide which implementation to use at compile time and keeping libgit2 in the codebase for now could make sense? What do you think? Too much work?

[blinxen]

What do you think? Too much work?

I don't see a benefit in this. I initially started this PR to reduce maintenance work on bat in Fedora. So if we are going to have the gitoxide feature then I would like to use it in Fedora. That however will make people create issues here that on Fedora they see a different diff output. Therefore I suggest to close this PR and maybe come back again later when the whole situation is better.

[Byron]

This makes me sad, but I also have no solution for the diff-slider issue and the Git related modifications. It's notable though that the version of this that helix is using was basically conjured up over night, with no tests that I know of. And I don't think gitoxide will ever test that in production, so it's indeed better to hold back and see how the situation resolves itself.