From: Thiago Farina <tfransosi@gmail.com>
Date: Mon, 09 May 2011 15:39:39 GMT
Subject: Re: [PATCH 2/3] blame: refactor porcelain output
Message-ID: <BANLkTi=EvgRsrULEFiZsOOM4cdfVFATcSg@mail.gmail.com>
In-Reply-To: <20110509133402.GB11022@sigill.intra.peff.net>

On Mon, May 9, 2011 at 10:34 AM, Jeff King <peff@peff.net> wrote:
> This is in preparation for adding more porcelain output
> options. The three changes are:
>
>  1. emit_porcelain now receives the format option flags
>
>  2. emit_one_suspect_detail takes an optional "repeat"
>     parameter to suppress the "show only once" behavior
>
>  3. The code for emitting porcelain suspect is factored
>     into its own function for repeatability.
>
> There should be no functional changes.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> I broke this out for readability. I can break each of the 3 out into a
> separate patch if that helps, but it seemed excessive.
>
>  builtin/blame.c |   25 ++++++++++++++++---------
>  1 files changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 4242e4b..d74e18f 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -1484,13 +1484,14 @@ static void write_filename_info(const char *path)
>  /*
>  * Porcelain/Incremental format wants to show a lot of details per
>  * commit.  Instead of repeating this every line, emit it only once,
> - * the first time each commit appears in the output.
> + * the first time each commit appears in the output (unless the
> + * user has specifically asked for us to repeat).
>  */
> -static int emit_one_suspect_detail(struct origin *suspect)
> +static int emit_one_suspect_detail(struct origin *suspect, int repeat)
>  {
>        struct commit_info ci;
>
> -       if (suspect->commit->object.flags & METAINFO_SHOWN)
> +       if (!repeat && suspect->commit->object.flags & METAINFO_SHOWN)

Maybe would be worth adding parentheses here:

if (!repeat && (...))
  return 0;

?

Probably is fine as is though.