Skip to content

Use gitoxide in get_status #2673

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

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

cruessler
Copy link
Collaborator

@cruessler cruessler commented Jun 29, 2025

This is a draft PR. I think that, at this point, most of the initial work of porting is done. I’m going to be using this branch as my daily driver throughout the week to find any issues there might still be left. I’m planning on marking it as ready for review at the end of the week.

This PR changes asyncgit::sync::get_status to use gitoxide. As a side-effect, it lets gitoxide handle reading status.showUntrackedFiles. The resulting behaviour, with respect to status.showUntrackedFiles, is closer to what git does than it was before this PR. I’ve updated 2 tests according to the new behaviour.

@cruessler cruessler force-pushed the use-gitoxide-for-status branch from 2916a78 to 1b3694b Compare June 29, 2025 15:06
let path = item.location().to_string();

match item {
gix::status::Item::IndexWorktree(_) => {}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@Byron Context: this is moving the status view in gitui to gitoxide, in particular the staging area.

There is tree_index_status which seems like something I’d want to use for this purpose, but the docs explicitly say: “This is a low-level method - prefer the Repository::status() platform instead”. Reading the docs for status, I didn’t find a way to configure it to give the same information as tree_index_status. Am I missing anything?

Copy link

Choose a reason for hiding this comment

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

I think I see what the problem is.
There is iterators for everything, and for index-worktree changes, and both are used.

But there is no iterator version for tree-index changes, which is what you could be looking for, only this relatively low-level callback version. The reason for that is that this is so fast there isn't a need be smart about it. Ideally, there would be a version which just returns a Vector of these changes, but in absence of it you could just call that and build a Vec yourself in the callback, or extract the needed information right there.

There is no alternative, this one would have to be used.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks a lot, really helpful! I changed the implementation to use tree_index_status.

I noticed that there is no ChangeRef::location, so I’m now getting it via change_ref.fields().0. Would it make sense to add an explicit ChangeRef::location to gitoxide?

Copy link

Choose a reason for hiding this comment

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

Yes, definitely, particularly for consistency. Ideally it's also advertised in the fields() docs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I’ve implemented the suggestion in GitoxideLabs/gitoxide#2071!

With this change, when `get_status` is run in a sub-directory, it also
returns matches in sub-directories that don’t share a prefix with the
sub-directory it is being run in.
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