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

From: Junio C Hamano <[email-masked]>
Date: Fri, 12 Nov 2021 15:23:26 -0800

"Samuel Yvon via GitGitGadget" <[email-masked]> writes:

> The comment suggesting that the cache must be reset after run_status
> and before the editor being launched was added in ec84bd00,
> (git-commit: Refactor creation of log message., 2008-02-05). It is
> unclear why the run_status must be called *after* the cache reset.

An older thread cited earlier suspected that it is to reflect the
changes given to "git commit" proper, excluding whatever pre-commit
did, to the return value of run_status(), which becomes the value of
committable variable to answer "do we have anything to commit?".

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.

> 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.

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).

Thanks for working on the topic.