Re: v1.5.4 plans

From: Jeff King <peff@peff.net>
Date: Dec 11, 2007, 06:54 UTC
Message-ID: <20071211065400.GA25985@coredump.intra.peff.net>
In-Reply-To: <7vd4tdhgnd.fsf@gitster.siamese.dyndns.org>

On Mon, Dec 10, 2007 at 10:47:18PM -0800, Junio C Hamano wrote:

Show 11 quoted lines

> > We are duplicating the "!shown && ..." conditional branch from
> > log_tree_commit, which calls show_log. Why are we not calling show_log
> > instead of pretty_print_commit (I understand that show_log should end up
> > calling pretty_print_commit, but it is not immediately obvious that all
> > of the extra code in show_log is going to be ignored).
> 
> Exactly.  I think show_log() has become too complex, and when we want a
> oneline userformat, pretty-print-commit is more appropriate to use.
> Actually, when I re-review the code, I think the part should use
> format_commit_message() which is more to the point without any of the
> other "more generic" parameter pretty-print-commit takes.

Perhaps it would be even more readable to simply split the printing of the commit message and the diff. Tracking this bug was a bit confusing. Ideally, print_summary would just say:

  print_commit_message(commit);
  print_diff(commit, "^commit");

where obviously each of those takes a few lines to say properly. But the use of a combination "show the log and maybe the diff" function seems like a shell holdover, trying to avoid spawning too many processes.

-Peff