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
