Re: [PATCH] graph.c: make many functions static

[Adam Simpkins <adam@adamsimpkins.net>]

On Fri, Jun 20, 2008 at 12:37:00AM -0700, Junio C Hamano wrote:
> Adam Simpkins <adam@adamsimpkins.net> writes:
> 
> > On Thu, Jun 19, 2008 at 12:16:11PM -0700, Junio C Hamano wrote:
> >> しらいしななこ  <nanako3@lavabit.com> writes:
> > ...
> >> > +/* Internal API */
> >> > + ...
> >> > +static int graph_next_line(struct git_graph *graph, struct strbuf *sb);
> >> > +static void graph_padding_line(struct git_graph *graph, struct strbuf *sb);
> >> > +static void graph_show_strbuf(struct git_graph *graph, struct strbuf const *sb);
> >> 
> >> I think these are probably fine, not in the sense that nobody calls these
> >> functions _right now_ but in the sense that I do not foresee a calling
> >> sequence outside the graph.c internal that needs to call these directly,
> >> instead of calling graph_show_*() functions that use these.
> >
> > Documentation/technical/api-history-graph.txt should also be updated
> > to remove the discussion of these functions if they are no longer
> > publicly exposed.
> 
> Actually, I was expecting (not necessarily _hoping_) you to defend these
> public API by providing examples that illustrates when calling these from
> outside graph API implementation could be useful.

Ah.  I see :-)

I do think that graph_next_line() and graph_padding_line() are
potentially useful as public APIs, since they are more generic than
the other API functions that wrap them.  These functions both output
to a strbuf.  graph_show_oneline() and graph_show_padding() are simple
wrappers around these functions that print the resulting strbuf to
stdout.  graph_next_line() and graph_padding_line() will be needed by
anyone who wants to write the graph output somewhere other than
stdout.  They could also be useful if someone wants to further
manipulate the output before printing it.

graph_show_strbuf() is a somewhat similar situation.
graph_show_commit_msg() is a wrapper around graph_show_strbuf()--it
calls graph_show_strbuf() and then also prints the remainder of the
graph.  A caller would need to use graph_show_strbuf() directly if
they have multiple strbufs that need to be displayed alongside the
graph.  In this case, they'll want to call graph_show_strbuf()
multiple times before printing the remainder of the graph.  (The
caller could also work around this by concatenating the strbufs first,
then passing the entire thing to graph_show_commit_msg().  However,
the downside of this approach is that it requires copying the
strbufs.)

I didn't defend these earlier since I wasn't sure if it was simply
git's style to make functions static until there is a proven need for
them to be public.  At the moment, the existing log-tree.c code is
only using the wrappers around these functions.  I don't really see an
immediate need for the core git code to use any of these functions
directly, but they might of interest to people working on libifying
git.

-- 
Adam Simpkins
adam@adamsimpkins.net