Re: revision API design, was Re: [PATCH v4 02/10] rebase -i: generate the script via rebase--helper

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

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

>> See 66b2ed09 ("Fix "log" family not to be too agressive about
>> ...
>> ask the existing command line parser to set them for you.
>
> This is a very eloquent description of a problem with the API.

Yes, but ...

> The correct suggestion would be to fix the API, of course. Not to declare
> an interface intended for command-line parsing the internal API.
>
> Maybe the introduction of `pretty_given` was a strong hint at a design
> flaw to begin with, pointing to the fact that user_format is a singleton
> (yes, really, you can't have two different user_formats in the same Git
> process, what were we thinking)?

... this tells me that you do not understand the issue.  The
embarrasing but necessary pretty-given field was not about
user_format (let alone singleton-ness of it) at all.  It was about
the show_notes feature that wants to be on by default most of the
time, but needs to be defeated when the end user asked for certain
formats.

Quite frankly, I am not interested in listening to a proposal to
update API by a person who does not understand the issue in the API,
but that is a separate issue.  A more important thing is that the
update to "rebase -i" is important enough and we do not want to
delay it by introducing further dependency.  IOW, I do not want to
spend an extra development cycle or two to update the revision setup
API and have you rebase the series on top after the API update is
done.

The current API to hide such an embarrasing but necessary
implementation details from the code that does not need to know
them, i.e. the consumer of rev-info structure with various settings,
is to invoke the command line parser.  Bypassing and going directly
down to the internal implementation detail of rev_info _is_ being
sloppy.  I would strongly prefer to see that the current series
written for the API to allow us get it over with the "rebase -i"
thing, and think revision setup API separately and later.