Hi Junio,
On Tue, 30 May 2017, Junio C Hamano wrote:
> We do not want these [revision API] implementation details to code that
> does not implement command line parsing. This one is not parsing
> anybody's set of options and should not be mucking with the low level
> implementation details.
Just to make sure we are on the same level: you want the argc & argv to be
the free-form API of setup_revisions(), rather than code setting fields
in struct rev_info whose names can be verified at compile time, and whose
names also suggest their intended use.
I am still flabberghasted that any seasoned software developer sincerely
would suggest this.
> See 66b2ed09 ("Fix "log" family not to be too agressive about
> showing notes", 2010-01-20) and poinder. Back then, nobody outside
> builtin/log.c and revision.c (these are the two primary things that
> implement command line parsing; "log.c" needs access to the low
> level details because it wants to establish custom default that is
> applied before it reads options given by the end-user) mucked
> directly with verbose_header, which allowed the addition of
> "pretty_given" to be limited only to these places that actually do
> the parsing. If the above patch to sequencer.c existed before
> 66b2ed09, it would have required unnecessary change to tweak
> "pretty_given" in there too when 66b2ed09 was done. That is forcing
> a totally unnecesary work. And there is no reason to expect that
> the kind of change 66b2ed09 made to the general command line parsing
> will not happen in the future. Adding more code like the above that
> knows the implementation detail is unwarranted, when you can just
> ask the existing command line parser to set them for you.
This is a very eloquent description of a problem with the API.
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)?
Ciao,
Dscho