Fix skipping a file when staging last file in a directory #2767
Conversation
Tillerino
force-pushed
the
binary-search-error-result
branch
from
8409e2c to
01815be
Compare
|
@Tillerino could you fix the conflicts? |
cruessler
left a comment
cruessler
left a comment
There was a problem hiding this comment.
I really like this change as that is something that has been bothering me for quite some time now. Thanks for the fix! I tested the fix briefly and did not find any issues. I left two minor comments, but nothing major.
| }); | ||
| match res { | ||
| Ok(i) => Some(i), | ||
| Err(i) => Some(cmp::min(i, self.tree.len() - 1)), |
There was a problem hiding this comment.
Will this ever pick self.tree.len() - 1 or could we just return i? Can we add a test case specifically for the former scenario if this can happen? (I can’t tell whether this is covered by existing tests already.)
Also, last_index is now an unused parameter. Can it be removed.
|
|
||
| #[test] | ||
| fn test_next_when_dir_disappears() { | ||
| let mut res = StatusTree::default(); |
There was a problem hiding this comment.
What do you think about renaming res to tree for clarity? (res probably comes from existing tests, but tree carries more context, so I think is better here).
3 participants
This Pull Request partly addresses #2748.
The "error" result from the binary search was not taken into account after an update to the status tree, which lead to files being skipped when the last file in a directory was staged.
Before:
Screencast_20251029_204232.webm
(skips
zzz)After:
Screencast_20251029_204327.webm
I followed the checklist:
make checkwithout errors