Re: [PATCH v7 2/7] graph: add support for --line-prefix on all graph-aware output

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Aug 17, 2016 at 5:51 PM, Jacob Keller <jacob.e.keller@xxxxxxxxx> wrote:
> From: Jacob Keller <jacob.keller@xxxxxxxxx>
>
> Add an extension to git-diff and git-log (and any other graph-aware
> displayable output) such that "--line-prefix=<string>" will print the
> additional line-prefix on every line of output.
>
> To make this work, we have to fix a few bugs in the graph API that force
> graph_show_commit_msg to be used only when you have a valid graph.
> Additionally, we extend the default_diff_output_prefix handler to work
> even when no graph is enabled.
>
> This is somewhat of a hack on top of the graph API, but I think it
> should be acceptable here.
>
> This will be used by a future extension of submodule display which
> displays the submodule diff as the actual diff between the pre and post
> commit in the submodule project.
>
> Add some tests for both git-log and git-diff to ensure that the prefix
> is honored correctly.
>
> Signed-off-by: Jacob Keller <jacob.keller@xxxxxxxxx>
> ---
>  Documentation/diff-options.txt                     |   3 +
>  builtin/rev-list.c                                 |  70 ++---
>  diff.c                                             |   7 +
>  diff.h                                             |   2 +
>  graph.c                                            | 104 ++++---
>  graph.h                                            |  22 +-
>  log-tree.c                                         |   5 +-
>  t/t4013-diff-various.sh                            |   6 +
>  ...diff.diff_--line-prefix=abc_master_master^_side |  29 ++
>  t/t4013/diff.diff_--line-prefix_--cached_--_file0  |  15 +
>  t/t4202-log.sh                                     | 323 +++++++++++++++++++++
>  11 files changed, 506 insertions(+), 80 deletions(-)
>  create mode 100644 t/t4013/diff.diff_--line-prefix=abc_master_master^_side
>  create mode 100644 t/t4013/diff.diff_--line-prefix_--cached_--_file0
>
> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt
> index 705a87394200..cc4342e2034d 100644
> --- a/Documentation/diff-options.txt
> +++ b/Documentation/diff-options.txt
> @@ -569,5 +569,8 @@ endif::git-format-patch[]
>  --no-prefix::
>         Do not show any source or destination prefix.
>
> +--line-prefix=<prefix>::
> +       Prepend an additional prefix to every line of output.
> +
>  For more detailed explanation on these common options, see also
>  linkgit:gitdiffcore[7].
> diff --git a/builtin/rev-list.c b/builtin/rev-list.c
> index 0ba82b1635b6..1a75a83538f4 100644
> --- a/builtin/rev-list.c
> +++ b/builtin/rev-list.c
> @@ -122,48 +122,40 @@ static void show_commit(struct commit *commit, void *data)
>                 ctx.fmt = revs->commit_format;
>                 ctx.output_encoding = get_log_output_encoding();
>                 pretty_print_commit(&ctx, commit, &buf);
> -               if (revs->graph) {
> -                       if (buf.len) {
> -                               if (revs->commit_format != CMIT_FMT_ONELINE)
> -                                       graph_show_oneline(revs->graph);
> +               if (buf.len) {
> +                       if (revs->commit_format != CMIT_FMT_ONELINE)
> +                               graph_show_oneline(revs->graph);
>
> -                               graph_show_commit_msg(revs->graph, &buf);
> +                       graph_show_commit_msg(revs->graph, stdout, &buf);
>
> -                               /*
> -                                * Add a newline after the commit message.
> -                                *
> -                                * Usually, this newline produces a blank
> -                                * padding line between entries, in which case
> -                                * we need to add graph padding on this line.
> -                                *
> -                                * However, the commit message may not end in a
> -                                * newline.  In this case the newline simply
> -                                * ends the last line of the commit message,
> -                                * and we don't need any graph output.  (This
> -                                * always happens with CMIT_FMT_ONELINE, and it
> -                                * happens with CMIT_FMT_USERFORMAT when the
> -                                * format doesn't explicitly end in a newline.)
> -                                */
> -                               if (buf.len && buf.buf[buf.len - 1] == '\n')
> -                                       graph_show_padding(revs->graph);
> -                               putchar('\n');
> -                       } else {
> -                               /*
> -                                * If the message buffer is empty, just show
> -                                * the rest of the graph output for this
> -                                * commit.
> -                                */
> -                               if (graph_show_remainder(revs->graph))
> -                                       putchar('\n');
> -                               if (revs->commit_format == CMIT_FMT_ONELINE)
> -                                       putchar('\n');
> -                       }
> +                       /*
> +                               * Add a newline after the commit message.

I wondered if it is my webmailer displaying things wrongly here so I checked
it at public inbox, and fetched the mails and applied them.

It seems to me as if this long comment is broken in indentation
(i.e. you removed a blank ' ' directly in front of the '*' instead of a '\t' ?)


> +                               *
> +                               * Usually, this newline produces a blank
> +                               * padding line between entries, in which case
> +                               * we need to add graph padding on this line.
> +                               *
> +                               * However, the commit message may not end in a
> +                               * newline.  In this case the newline simply
> +                               * ends the last line of the commit message,
> +                               * and we don't need any graph output.  (This
> +                               * always happens with CMIT_FMT_ONELINE, and it
> +                               * happens with CMIT_FMT_USERFORMAT when the
> +                               * format doesn't explicitly end in a newline.)
> +                               */

> - if (!graph)
> + if (!graph) {
> +         graph_show_line_prefix(default_diffopt);
>           return;
> + }

> -       if (graph_is_commit_finished(graph))
> +       if (graph_is_commit_finished(graph)) {
> +               graph_show_line_prefix(&graph->revs->diffopt);
>                 return 0;
> +       }

This seems to be a reoccurring pattern, i.e. we need to add a lot of
one off instructions before a return. Is it possible to make the call
earlier instead
of just before the returns? (or later for that matter) ?


Thanks,
Stefan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]