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

2 messages, 2 authors, 2021-11-09 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2021-11-09 18:36:37

Samuel Yvon [off-list ref] writes:
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@gitster.c.googlers.com/t/#u:
quoted
quoted
Junio C Hamano [off-list ref] 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.
You seem to be quoting the thread over and over, but what you are
quoting is somewhat different from the concluding part of what I
said, which was:

    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.

    So, in short, it is not "undefined", but rather it seems to be a
    designed behaviour that we are seeing.

I do not personally mind if we change the philosophy but because it
has been a longstanding designed behaviour, it may need a careful
transition plan.

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

From: Samuel Yvon <hidden>
Date: 2021-11-09 20:02:10

Junio C Hamano [off-list ref] writes:
You seem to be quoting the thread over and over, but what you are
quoting is somewhat different from the concluding part of what I
said, which was:
quoted
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.
So, in short, it is not "undefined", but rather it seems to be a
designed behaviour that we are seeing.
Apologies if you feel I misquoted you. I am confused by the comment in the
original code w.r.t the location of the cache reset, which (from my
understanding) is contradictory with the quoted conversation and so I
focused on those parts.

Junio C Hamano [off-list ref] writes:
I do not personally mind if we change the philosophy but because it
has been a longstanding designed behaviour, it may need a careful
transition plan.
Out of curiosity, what would that involve? 
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help