On Fri, May 16, 2025 at 7:55 AM Derrick Stolee via GitGitGadget <gitgitgadget@xxxxxxxxx> wrote: > [...] > Updates in v2 > ============= > > Thanks for the careful review from Elijah and the pointer from Phillip, we > have these changes: > > 1. The tests no longer have different expansion behaviors for 'git add -p' > and 'git add -i' due to partially-expanded indexes on disk. > 2. We now test 'git checkout -p' and 'git reset -p'. > 3. 'git reset -p' needed some changes to the builtin (similar to 'git add') > to be fast. I haven't looked at the new patch yet, but the updated patches 1 & 2 look much improved (though there's still one problem with the new commit message, which I comment on in the range-diff below). However, I think Junio already merged your v1 to next (https://lore.kernel.org/git/CABPp-BEukTWwsuC7MMR8D5_UAhyw-LgT=DsPKAWeR_ZmVVhjzQ@xxxxxxxxxxxxxx/). So he'll either have to revert your v1 in next and apply the new series on top, or you'll need to re-roll as fixes on top of your v1. > 2: 63caae87634 ! 2: 0a2752721d0 git add: make -p/-i aware of sparse index > @@ Commit message > > It turns out that control flows out of cmd_add() in the interactive > cases before the lines that confirm that the builtin is integrated with > - the sparse index. We need to move that earlier to ensure it prevents a > - full index expansion on read. > + the sparse index. > > - Add more test cases that confirm that these interactive add options work > - with the sparse index. One interesting aspect here is that the '-i' > - option avoids expanding the sparse index when a sparse directory exists > - on disk while the '-p' option does hit the ensure_full_index() method. > - This leaves some room for improvement, but this case should be atypical > - as users should remain within their sparse-checkout. > + Moving that integration point earlier in cmd_add() allows 'git add -p' > + and 'git add -p' to operate without expanding a sparse index to a full > + one. Was the second 'git add -p' meant to be 'git add -i'? > -: ----------- > 3: d1482a29d8f reset: integrate sparse index with --patch Other than the one comment above, your changes from the range-diff look good to me for patches 1 & 2, and the new 4. I haven't looked at this new patch 3 yet but wanted to comment on the merged-to-next issue.
