Re: [PATCH v2] builtin-commit: re-read file index before run_status

Apologies for the time I took to reply,

Junio C Hamano [off-list ref] writes:

And moving the call would affect both the contents of the status
buffer (i.e. the list of paths got changed starts including what
        pre-commit did) and the "committable" bit by counting such a change
as a true change, avoiding the "no empty commit by default" check,
in a consistent way, hopefully.  I wonder if we have test to
demonstrate that, and if there isn't perhaps we would want to add
one.

So, just to make sure I understand well, the concern is that an empty commit
would trigger the commit routine, run the pre-commit hook, which may add files
(thus making it an non-empty commit) and then push 100% automatic changes to a
repo. I agree that this would be invalid behaviour and very odd, I will 
make sure no empty commit is allowed.

Junio C Hamano [off-list ref] writes:

Samuel Yvon [off-list ref] writes:

quoted

However, calling run_status after the cache reset does not update
the status line to state of the current index in the case a
pre-commit hook is ran and changes files in the staging area.

And if this change also affects the "committable" assignment in a
consistent way, it should probably want to be mentioned in this
paragraph, too.

What do you mean by "commitable assignment"? 

I am not convinced by the claim that there is no need for careful
transition plans (yet), but I personally agree with the end state
(with the above suggested tweaks, that is).

With the last message, I agree the safest option is probably to leave this
configurable for now and off by default.

So here's the next steps that I intend to take to get this merged in:

- Add a test for empty commit (if non-existent) and ensure the behaviour is the same
- Add a config option (or maybe a switch?) for migration purposes, with the default
  being the current behaviour.