From: Phillip Wood <phillip.wood123@gmail.com>
Date: Tue, 05 Sep 2023 15:25:25 GMT
Subject: Re: [PATCH v3 6/7] rebase --continue: refuse to commit after failed command
Message-ID: <84227916-92d9-4085-9738-29d7e489f144@gmail.com>
In-Reply-To: <87fbc8c9-f42b-b374-fee1-57c58f5e8fc0@gmx.de>

Hi Johannes

On 05/09/2023 12:17, Johannes Schindelin wrote:
> A further contributing factor for my misunderstading was the slightly
> convoluted logic where `is_clean` is set to true if there are _not_ any
> uncommitted changes, and then we ask if `is_clean` is _not_ true. Reminds
> me of Smullyan's Knights & Knaves [*1*].

I agree 'is_clean' is confusing (I have the same problem with 
merge_recursive() and friends where a return value of zero means that 
there were conflicts)

> With your patch, there are now four users of the `is_clean` value, and
> all but one of them ask for the negated value.
> 
> It's not really the responsibility of this patch series, but I could
> imagine that it would be nicer to future readers if a patch was added that
> would invert the meaning of that variable and rename it to
> `needs_committing`. At least to me, that would make the intention of the
> code eminently clearer.

Inverting and renaming 'is_clean' is a good idea, I might leave it to a 
follow up series though.

Best Wishes

Phillip

> Ciao,
> Johannes
> 
> Footnote *1*: https://en.wikipedia.org/wiki/Knights_and_Knaves