Re: obsolete index in wt_status_print after pre-commit hook runs

Am 04.08.2016 um 12:45 nachm. schrieb Junio C Hamano [off-list ref]:

Andrew Keller [off-list ref] writes:

quoted

In summary, I think I prefer #2 from a usability point of view, however I’m having
trouble proving that #1 is actually *bad* and should be disallowed.

Yeah, I agree with your argument from the usability and safety point
of view.

quoted

Any thoughts?  Would it be better for the pre-commit hook to be
officially allowed to edit the index [1], or would it be better
for the pre-commit hook to explicitly *not* be allowed to edit the
index [2], or would it be yet even better to simply leave it as it
is?

It is clear that our stance has been the third one so far.

Another thing I did not see in your analysis is what happens if the
user is doing a partial commit, and how the changes made by
pre-commit hook is propagated back to the main index and the working
tree.

The HEAD may have a file with contents in the "original" state, the
index may have the file with "update 1", and the working tree file
may have it with "update 2".  After the commit is made, the user
will continue working from a state where the HEAD and the index have
"update 1", and the working tree has "update 2".  "git diff file"
output before and after the commit will be identical (i.e. the
difference between "update 1" and "update 2") as expected.

Excellent point — one I had discovered myself but neglected to include in
my email.  In my post-commit hook, I have logic in both versions of my
experiment that disallows [1] fixing up diffs that are partially staged.  Both
scripts then update both the index and the working copy.  (Sort of like how
rebase works — clean working directory required, and then it updates the
index and the work tree)

[1] In version #1, if any files it wants to change are partially staged, it
    prints a detailed error message and aborts the commit outright.  In
    version #2, the pre-commit hook sees the change it _wants_ to make,
    informs the user that he/she should run the fixup command, aborts
    the commit, and when the user runs the fixup command, the fixup
    command sees the partially staged file, prints the same detailed error
    message, and dies.

Thanks for your help on this.  it’s really been interesting.  I’ll leave it as-is
for now.

Thanks,
 - Andrew Keller