gitui icon indicating copy to clipboard operation
gitui copied to clipboard

Use `gitoxide` in `get_commits_info` and `get_commit_info`

Open cruessler opened this issue 8 months ago • 9 comments

  • Implement From<gix::ObjectId> for CommitId
  • Use gitoxide in get_commits_info and get_commit_info

This PR changes get_commits_info and get_commit_info to use gitoxide under the hood. It does not explicitly change any behaviour, although there possibly are subtle differences that I’m not aware of.

This implementation doesn’t log an error when either author or committer cannot be resolved using mailmap as the underlying implementation returns Option<Signature> instead of Result<Signature>. If you want, I can change that, though.

I decided to duplicate the logic of get_message into gix_get_message, using a prefix for the time both implementations are required.

I did not attempt to re-organize any of the code. My plan is to convert more functions to gitoxide before making any changes in that direction.

I followed the checklist:

  • [x] I ran make check without errors
  • [x] I tested the overall application

cruessler avatar May 17 '25 17:05 cruessler

@cruessler excited to see more gitoxide in the codebase. any chance to get the CI green?

extrawurst avatar May 20 '25 09:05 extrawurst

@cruessler excited to see more gitoxide in the codebase. any chance to get the CI green?

I’m having a look!

cruessler avatar May 20 '25 13:05 cruessler

So far, I have not been able to reproduce the test failures reliably on my machine. My current best guess is that they are somehow timing-related, but I don’t yet know why that might be the case as the code in questions seems to be sync. I think it is this call that triggers the error in rare circumstances, but that’s all I got:

https://github.com/gitui-org/gitui/blob/d8eb1f353f03c0cb4b1eccd536e930832de37729/asyncgit/src/sync/blame.rs#L97

cruessler avatar May 22 '25 08:05 cruessler

I’m currently working on adding rename tracking to gitoxide blame: GitoxideLabs/gitoxide#2022. Instead of trying to debug the CI failures we hit in this PR, another option would be to wait for the PR in gitoxide to land, then port gitui’s blame view to gitoxide and then retry porting get_commits_info to gitoxide.

cruessler avatar Jun 01 '25 09:06 cruessler

@cruessler whatever works best for you

extrawurst avatar Jun 01 '25 10:06 extrawurst

@extrawurst Since rename tracking for blame has landed in gitoxide a few days ago, I think I’m leaning towards making the switch for blame first.

cruessler avatar Jun 10 '25 11:06 cruessler

Sounds good to me

extrawurst avatar Jun 10 '25 12:06 extrawurst

@cruessler should we mark this draft while we wait for blame to land?

extrawurst avatar Nov 28 '25 21:11 extrawurst

@cruessler should we mark this draft while we wait for blame to land?

Makes total sense, I’ve converted it to draft! (I hope I’m not spamming too much with all these experimental PRs that explore using gitoxide in various parts of gitui.)

cruessler avatar Nov 29 '25 07:11 cruessler