Re: [PATCH v3 6/7] rebase --continue: refuse to commit after failed command

[Junio C Hamano <gitster@pobox.com>]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> With your patch, there are now four users of the `is_clean` value, and
> all but one of them ask for the negated value.

Excellent observation.  That strongly argues for the flipping of
polarity, i.e. many people want to know "is it unclean/dirty?".  It
is funny that the name of the helper function where the value comes
from, i.e. has_uncommitted_changes(), has the desired polarity.

> 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.

While I agree, after reading the code, that it would make it easier
to follow to flip the polarity of the variable, I would advise
against renaming from state based naming (is it dirty?) to action
based naming (must we commit?), *if* the variable is checked to
sometimes see if we has something that we _could_ commit, while some
other times to see if we _must_ commit before we can let the user
proceed.

"Does the index hold some changes to be committed?" is better
question than "Must we commit?" or "Could we commit?" to derive the
name of this variable from if that is the case, I would think.