Re: [PATCH v7 2/7] graph: add support for --line-prefix on all graph-aware output
- From
Stefan Beller <sbeller@google.com>
- Date
- Aug 18, 2016, 17:56 UTC
- Message-ID
- <CAGZ79kZeSwteU84-=GZ0cZDCJUf6=VrGzAiNqC_gx7DgTQX-8Q@mail.gmail.com>
- In-Reply-To
- <20160818005131.31600-3-jacob.e.keller@intel.com>
On Wed, Aug 17, 2016 at 5:51 PM, Jacob Keller <jacob.e.keller@intel.com> wrote:
Show 101 quoted lines
> From: Jacob Keller <jacob.keller@gmail.com>
>
> 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@gmail.com>
> ---
> 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' ?)
Show 13 quoted lines
> + * > + * 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.) > + */
Show 5 quoted lines
> - if (!graph)
> + if (!graph) {
> + graph_show_line_prefix(default_diffopt);
> return;
> + }Show 5 quoted lines
> - 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