Subject: Re: [PATCH 5/8] rebase: preserve interactive todo file on checkout failure

From: Junio C Hamano <[email-masked]>
Date: Thu, 23 Mar 2023 21:31:29 -0700

Oswald Buddenhagen <[email-masked]> writes:

> On Thu, Mar 23, 2023 at 01:16:47PM -0700, Junio C Hamano wrote:
>>Oswald Buddenhagen <[email-masked]> writes:
>>
>>> Creating a suitable todo file is a potentially labor-intensive process,
>>> so be less cavalier about discarding it when something goes wrong (e.g.,
>>> the user messed with the repo while editing the todo).
>>
>>Is there a reason why we do not always keep it?  Why is the file
>>sometimes precious but not precious at all in other times?
>>
> the unedited initial todo just isn't precious. that implies that in a
> non-interactive rebase, it is always worthless at the time of the
> initial reset.

I see.  Thanks for clarifying.

Just FYI, the primary purpose reviewers ask questions on the
proposed change is to help submitters polish their patch (both the
proposed log message text and the code) to clarify points they found
hard to understand and/or they suspect would be hard to understand
for other readers.  So please do not be happy by just receiving "I
see, thanks" and stop there.  Instead, please update the patch so
that future readers would not have to ask similar question again.

>>(and if you can reliably ensure that the file has contents
>>that are expected, that would be even better)?
>>
> i could grep for a shortened sha1 i would obtain from the branch. but
> given that the error scenario of a present but somehow corrupted todo
> seems implausible given the circumstances, that seems like overkill.

It is OK.  If it were easy to prepare the "todo should look like
this" golden copy, then doing test_cmp the actual file with it would
have been a simple way to ensure both existence of and sane contents
in the file at the same time, but if it isn't cheap to prepare such
an expected output, I agree with you that it is not worth the extra
effort.

Thanks.