Conversation
b6feeb6 to
a942922
Compare
a942922 to
68b0e42
Compare
src/info/pending.rs
Outdated
| Summary::Removed => (added, deleted + 1, modified), | ||
| Summary::Added => (added + 1, deleted, modified), | ||
| Summary::Modified | Summary::TypeChange => (added, deleted, modified + 1), | ||
| Summary::Renamed | Summary::Copied => (added + 1, deleted + 1, modified), |
There was a problem hiding this comment.
Watching the conclusion of switching from git2 to gix is awesome!
Should deleted really be incremented when the status is Copied? AFAIK a rename is always a delete + add, while a copy just means a new file was created with extremely similar contents to an existing file.
There was a problem hiding this comment.
That's a great catch right there! Copied should only count as added file.
This Summary was added just for onefetch and it was interesting to see that the possible states indeed are best represented by an enum, even though libgit2 tries to convince you otherwise.
68b0e42 to
dfc0831
Compare
|
While I prepare the Details |
| Summary::Removed => (added, deleted + 1, modified), | ||
| Summary::Added | Summary::Copied => (added + 1, deleted, modified), | ||
| Summary::Modified | Summary::TypeChange => (added, deleted, modified + 1), | ||
| Summary::Renamed => (added + 1, deleted + 1, modified), |
There was a problem hiding this comment.
By the way, it won't run into Renamed and Copied as rename tracking isn't activated by default.
There was a problem hiding this comment.
Can you expand on that, what are the implications ?
There was a problem hiding this comment.
This would mean the code could call unreachable!() in case of Copied and Renamed and be right about it. The implementation/settings have the same effect as git2 previously.
Does that make more sense?
There was a problem hiding this comment.
Are you refering to git config --get diff.renames ? Whether it's enable doesn't change the output for onefetch, right? a rename is always shown as an addition and an deletion.
There was a problem hiding this comment.
That's a great point to bring up! I decided not to let it affect renames between index and worktree as Git itself doesn't even expose that functionality to the user (only git2 seems to have it). Everything that is affected by configuration, really only emit_untracked at this time, is overridden though.
dfc0831 to
9c15105
Compare
|
Great to hear that another issue is fixed :). This PR is good to go from my side. |
Fixes #628
I had to wait a long time for this PR to become possible, and finally here it is :)!
Tasks
git2dependencygixReview Notes
onefetchtruly read-only.If there is disagreement, the index could be updated and written back - it will be valid, but won't write back all extensions that might
have been present before.
git2most certainly also didn't do it in a way that is 100% faithful to what Git would do.renames_head_to_index()was never functional, as the status was only obtained between the index and the worktree. If this must be presentwith the switch, then I have to hold off with this update as
gix statusdoesn't yet supportHEAD->indexstatus.Performance
In short,
onefetchgot a little bit faster, and speeds up with the size of the repositories measured by the amount of files in their worktrees.WebKit
Here the faster
statusimplementation is definitely visible.Linux
For linux, the commit-aggregation is the most stressful part, and there is much less of a difference as
statusis just a small part of the overall runtime.Git
Git is interesting as it doesn't have too many files in the worktree, yet it seems to greatly benefit from the new
statusimplementation. Unfortunately, at ~200ms, the difference isn't really perceivable by a user.