Re: [RFC/PATCH 10/17] submodule.c: convert show_submodule_summary to use emit_line_fmt

2016-09-13 Thread Stefan Beller
On Tue, Sep 13, 2016 at 4:02 PM, Junio C Hamano  wrote:
> Stefan Beller  writes:
>
>> In a later patch, I want to propose an option to detect&color
>> moved lines in a diff, which cannot be done in a one-pass over
>> the diff. Instead we need to go over the whole diff twice,
>> because we cannot detect the first line of the two corresponding
>> lines (+ and -) that got moved.
>>
>> So to prepare the diff machinery for two pass algorithms
>> (i.e. buffer it all up and then operate on the result),
>> move all emissions to places, such that the only emitting
>> function is emit_line_0.
>>
>> This prepares the code for submodules to go through the
>> emit_line_0 function.
>>
>> Signed-off-by: Stefan Beller 
>> ---
>
> I wonder how this interacts with the jk/diff-submodule-diff-inline
> topic by Jacob that has graduated recently to 'master'.  IIRC, it
> just lets a separate "git diff" instance that is spawned in the
> submodule directory emit its findings to the output of the driving
> "git diff" in the superproject.
>

The easiest way to find out is to merge HEAD^ of this patch series
(i.e. "diff: buffer output in emit_line_0") with whatever we suspect can
cause breakage for the goal of channeling everything though emit_line_*
functions. Looking at that series, I think I'll have to redo
this (maybe even including sb/diff-cleanup, to have it all in one series)
to capture all output there.

I suspected a breakage, but as the patch series grew larger and larger,
I first wanted to get into a working state before paying attention to solving
conflicts as resolving conflicts is easier when I know where this series is
headed.

Thanks!
Stefan


Re: [RFC/PATCH 10/17] submodule.c: convert show_submodule_summary to use emit_line_fmt

2016-09-13 Thread Junio C Hamano
Stefan Beller  writes:

> In a later patch, I want to propose an option to detect&color
> moved lines in a diff, which cannot be done in a one-pass over
> the diff. Instead we need to go over the whole diff twice,
> because we cannot detect the first line of the two corresponding
> lines (+ and -) that got moved.
>
> So to prepare the diff machinery for two pass algorithms
> (i.e. buffer it all up and then operate on the result),
> move all emissions to places, such that the only emitting
> function is emit_line_0.
>
> This prepares the code for submodules to go through the
> emit_line_0 function.
>
> Signed-off-by: Stefan Beller 
> ---

I wonder how this interacts with the jk/diff-submodule-diff-inline
topic by Jacob that has graduated recently to 'master'.  IIRC, it
just lets a separate "git diff" instance that is spawned in the
submodule directory emit its findings to the output of the driving
"git diff" in the superproject.



[RFC/PATCH 10/17] submodule.c: convert show_submodule_summary to use emit_line_fmt

2016-09-12 Thread Stefan Beller
In a later patch, I want to propose an option to detect&color
moved lines in a diff, which cannot be done in a one-pass over
the diff. Instead we need to go over the whole diff twice,
because we cannot detect the first line of the two corresponding
lines (+ and -) that got moved.

So to prepare the diff machinery for two pass algorithms
(i.e. buffer it all up and then operate on the result),
move all emissions to places, such that the only emitting
function is emit_line_0.

This prepares the code for submodules to go through the
emit_line_0 function.

Signed-off-by: Stefan Beller 
---
 diff.c  |  5 ++---
 diff.h  |  3 +++
 submodule.c | 42 +-
 submodule.h |  3 +--
 4 files changed, 23 insertions(+), 30 deletions(-)

diff --git a/diff.c b/diff.c
index 3aa99d1..1892c2b 100644
--- a/diff.c
+++ b/diff.c
@@ -493,7 +493,7 @@ static void emit_line(struct diff_options *o, const char 
*set, const char *reset
emit_line_0(o, set, reset, line[0], line+1, len-1);
 }
 
-static void emit_line_fmt(struct diff_options *o,
+void emit_line_fmt(struct diff_options *o,
  const char *set, const char *reset,
  const char *fmt, ...)
 {
@@ -2306,8 +2306,7 @@ static void builtin_diff(const char *name_a,
(!two->mode || S_ISGITLINK(two->mode))) {
const char *del = diff_get_color_opt(o, DIFF_FILE_OLD);
const char *add = diff_get_color_opt(o, DIFF_FILE_NEW);
-   show_submodule_summary(o->file, one->path ? one->path : 
two->path,
-   line_prefix,
+   show_submodule_summary(o, one->path ? one->path : two->path,
one->oid.hash, two->oid.hash,
two->dirty_submodule,
meta, del, add, reset);
diff --git a/diff.h b/diff.h
index 7883729..1699d9c 100644
--- a/diff.h
+++ b/diff.h
@@ -180,6 +180,9 @@ struct diff_options {
int diff_path_counter;
 };
 
+void emit_line_fmt(struct diff_options *o, const char *set, const char *reset,
+  const char *fmt, ...);
+
 enum color_diff {
DIFF_RESET = 0,
DIFF_CONTEXT = 1,
diff --git a/submodule.c b/submodule.c
index 1b5cdfb..c817b46 100644
--- a/submodule.c
+++ b/submodule.c
@@ -304,8 +304,8 @@ static int prepare_submodule_summary(struct rev_info *rev, 
const char *path,
return prepare_revision_walk(rev);
 }
 
-static void print_submodule_summary(struct rev_info *rev, FILE *f,
-   const char *line_prefix,
+static void print_submodule_summary(struct rev_info *rev,
+   struct diff_options *o,
const char *del, const char *add, const char *reset)
 {
static const char format[] = "  %m %s";
@@ -317,24 +317,16 @@ static void print_submodule_summary(struct rev_info *rev, 
FILE *f,
ctx.date_mode = rev->date_mode;
ctx.output_encoding = get_log_output_encoding();
strbuf_setlen(&sb, 0);
-   strbuf_addstr(&sb, line_prefix);
-   if (commit->object.flags & SYMMETRIC_LEFT) {
-   if (del)
-   strbuf_addstr(&sb, del);
-   }
-   else if (add)
-   strbuf_addstr(&sb, add);
format_commit_message(commit, format, &sb, &ctx);
-   if (reset)
-   strbuf_addstr(&sb, reset);
-   strbuf_addch(&sb, '\n');
-   fprintf(f, "%s", sb.buf);
+   if (commit->object.flags & SYMMETRIC_LEFT)
+   emit_line_fmt(o, del, reset, "%s\n", sb.buf);
+   else if (add)
+   emit_line_fmt(o, add, reset, "%s\n", sb.buf);
}
strbuf_release(&sb);
 }
 
-void show_submodule_summary(FILE *f, const char *path,
-   const char *line_prefix,
+void show_submodule_summary(struct diff_options *o, const char *path,
unsigned char one[20], unsigned char two[20],
unsigned dirty_submodule, const char *meta,
const char *del, const char *add, const char *reset)
@@ -359,30 +351,30 @@ void show_submodule_summary(FILE *f, const char *path,
message = "(revision walker failed)";
 
if (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
-   fprintf(f, "%sSubmodule %s contains untracked content\n",
-   line_prefix, path);
+   emit_line_fmt(o, 0, 0,
+ "Submodule %s contains untracked content\n", 
path);
if (dirty_submodule & DIRTY_SUBMODULE_MODIFIED)
-   fprintf(f, "%sSubmodule %s contains modified content\n",
-   line_prefix, path);
+   emit_line_fmt(o, 0, 0,
+ "Submodule %s contains modified content\n", path);
 
if (!hashcmp(one, two)) {
s