Am 25.07.20 um 17:31 schrieb Junio C Hamano: > Junio C Hamano <gitster@xxxxxxxxx> writes: > >> Maxime Louet <maxime@xxxxxxxxx> writes: >> >>> Is this expected behaviour? I find it somehow confusing that the diff >>> in the commit message isn't the actual commit diff. > Even before ec84bd00 (git-commit: Refactor creation of log message., > 2008-02-05), the code anticipated that pre-commit may touch the index > and tried to cope with it. > > However, ec84bd00 moved the place where we re-read the on-disk index > in the sequence, and updated a message that used to read: > > - /* > - * Re-read the index as pre-commit hook could have updated it, > - * and write it out as a tree. > - */ > > to: > > + /* > + * Re-read the index as pre-commit hook could have updated it, > + * and write it out as a tree. We must do this before we invoke > + * the editor and after we invoke run_status above. > + */ When I read "refactor" in the title, I assume that the patch in question doesn't change user-visible behavior. > Unfortunately there is no mention of the reason why we "must" here. @Paolo: Do you perhaps remember the reason? > I think the "run_status above" is what prepared the patch in the log > message template, so it is quite likely that we deliberately did so > to exclude whatever munging pre-commit does to the index from > appearing in the patch in the verbose mode. If I have to guess, I > think the reason is because pre-commit automation is expected to be > some sort of mechanical change and not part of the actual work that > the end-user produced, it would become easier to perform the "final > review" of "what have I done so far---does everything make sense?" > if such "extra" changes are excluded. Committers review and sign off changes. Hiding machine-made extra changes from them, that they then implicitly also accept responsibility for sounds questionable to me. The prepare-commit-msg hook might be a place for such filtering. But git commit showing the full extent of changes (incl. those made by the pre-commit hook) would be a better default, wouldn't it? René
