Re: [PATCH v5 08/10] rebase -i: skip unnecessary picks using the rebase--helper

[Jeff King <peff@peff.net>]

On Mon, Jun 19, 2017 at 11:45:50AM +0200, Johannes Schindelin wrote:

> The reason for this suggestion is that one of the revision machinery's
> implementation details is an ugly little semi-secret: the pretty-printing
> machinery uses a global state, and that is why we need the "pretty_given"
> flag in the first place.

I think that's mis-stating Junio's complaint. The point is not the
pretty_given flag itself, which we know about and can work around. The
point is that we don't know what other similar problems we have or will
have due to future changes in the revision code.

In other words, there are two APIs: the one where C code manipulates
rev_info directly, and the one where revision.c responds to string
arguments. From a maintenance perspective, it is easy for somebody make
a change that works for the latter but not the former.

I do agree that the lack of compile-time safety for obvious mistakes
like "--pertty" is a downside, though. On the other hand, there are
strong run-time checks there, so the tests would catch it.

I do not have a strong opinion myself in either direction.

-Peff