Re: [PATCH] builtin-commit: re-read file index before launching editor

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
> *nod*, the implicit suggestion here being: Let's put more of that
> summary into the commit message. It helps when looking up/discovering
> older behavior.

Sorry! I will prepare an amended patch later. Exploring the linked commits
has raised more questions on my end.

> Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
> The comment was first added in 2888605c649 (builtin-commit: fix
>         partial-commit support, 2007-11-18), and quite suspicuous in timing is
> f5bbc3225c4 (Port git commit to C., 2007-11-08) where we moved from
> git-commit.sh.

Exploring these commits, I fear I might have been confused; the editor was
always launched after the reset (still today). The problem, as noted earlier,
is the fact that the run_status is ran before the cache reset, which
creates the outdated diff. I can't speculate to the intent of the
author, but exploring the code makes this comment:

> +    /*
> +     * 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.
> +     */

more confusing, because there is no point of invoking the editor after the reset
if the status is written before, since the editor won't show anything different.

On one hand, flushing then showing the editor seems to indicate that we want the
editor to be up-to-date, but because the status is prepared before the flushing,
maybe not?

While it seems the current behaviour has been the behaviour since the start,
I am inclined to continue pushing for this change. Unless I am missing something,
the comment is contradictory and we should be coherent with the idea of
accepting changes made within the pre-commit hook, as noted in 
https://lore.kernel.org/git/xmqqk0yripca.fsf@xxxxxxxxxxxxxxxxxxxxxx/t/#u:

>> Junio C Hamano <gitster@xxxxxxxxx> writes:
>> 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.



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux