* [PATCH 01/10] diff: move line ending check into emit_hunk_header
2016-09-11 5:42 [PATCH 00/10] Another preparatory series for rename detection Stefan Beller
@ 2016-09-11 5:42 ` Stefan Beller
2016-09-12 23:35 ` Junio C Hamano
2016-09-11 5:42 ` [PATCH 02/10] diff: emit_{add, del, context}_line to increase {pre,post}image line count Stefan Beller
` (8 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Stefan Beller @ 2016-09-11 5:42 UTC (permalink / raw)
To: gitster; +Cc: git, Stefan Beller
From: Stefan Beller <sbeller@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
diff.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/diff.c b/diff.c
index cc8e812..aa50b2d 100644
--- a/diff.c
+++ b/diff.c
@@ -610,6 +610,9 @@ static void emit_hunk_header(struct emit_callback *ecbdata,
}
strbuf_add(&msgbuf, line + len, org_len - len);
+ if (line[org_len - 1] != '\n')
+ strbuf_addch(&msgbuf, '\n');
+
emit_line(ecbdata->opt, "", "", msgbuf.buf, msgbuf.len);
strbuf_release(&msgbuf);
}
@@ -1247,8 +1250,6 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
len = sane_truncate_line(ecbdata, line, len);
find_lno(line, ecbdata);
emit_hunk_header(ecbdata, line, len);
- if (line[len-1] != '\n')
- putc('\n', o->file);
return;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 01/10] diff: move line ending check into emit_hunk_header
2016-09-11 5:42 ` [PATCH 01/10] diff: move line ending check into emit_hunk_header Stefan Beller
@ 2016-09-12 23:35 ` Junio C Hamano
0 siblings, 0 replies; 21+ messages in thread
From: Junio C Hamano @ 2016-09-12 23:35 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, Stefan Beller
Stefan Beller <stefanbeller@gmail.com> writes:
> From: Stefan Beller <sbeller@google.com>
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
The reason being...?
"Doing this would not change any behaviour and would not break
anything" may be true, but that is not a reason to do a change.
Hopefully it will become clear why this is needed once we look at a
later patch in the series.
> diff.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index cc8e812..aa50b2d 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -610,6 +610,9 @@ static void emit_hunk_header(struct emit_callback *ecbdata,
> }
>
> strbuf_add(&msgbuf, line + len, org_len - len);
> + if (line[org_len - 1] != '\n')
> + strbuf_addch(&msgbuf, '\n');
> +
> emit_line(ecbdata->opt, "", "", msgbuf.buf, msgbuf.len);
> strbuf_release(&msgbuf);
> }
> @@ -1247,8 +1250,6 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
> len = sane_truncate_line(ecbdata, line, len);
> find_lno(line, ecbdata);
> emit_hunk_header(ecbdata, line, len);
> - if (line[len-1] != '\n')
> - putc('\n', o->file);
> return;
> }
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 02/10] diff: emit_{add, del, context}_line to increase {pre,post}image line count
2016-09-11 5:42 [PATCH 00/10] Another preparatory series for rename detection Stefan Beller
2016-09-11 5:42 ` [PATCH 01/10] diff: move line ending check into emit_hunk_header Stefan Beller
@ 2016-09-11 5:42 ` Stefan Beller
2016-09-12 23:47 ` Junio C Hamano
2016-09-11 5:42 ` [PATCH 03/10] diff.c: drop tautologous condition in emit_line_0 Stefan Beller
` (7 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Stefan Beller @ 2016-09-11 5:42 UTC (permalink / raw)
To: gitster; +Cc: git, Stefan Beller
From: Stefan Beller <sbeller@google.com>
At all call sites of emit_{add, del, context}_line we increment the line
count, so move it into the respective functions to make the code at the
calling site a bit clearer.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
diff.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/diff.c b/diff.c
index aa50b2d..156c2aa 100644
--- a/diff.c
+++ b/diff.c
@@ -536,6 +536,7 @@ static void emit_add_line(const char *reset,
struct emit_callback *ecbdata,
const char *line, int len)
{
+ ecbdata->lno_in_postimage++;
emit_line_checked(reset, ecbdata, line, len,
DIFF_FILE_NEW, WSEH_NEW, '+');
}
@@ -544,6 +545,7 @@ static void emit_del_line(const char *reset,
struct emit_callback *ecbdata,
const char *line, int len)
{
+ ecbdata->lno_in_preimage++;
emit_line_checked(reset, ecbdata, line, len,
DIFF_FILE_OLD, WSEH_OLD, '-');
}
@@ -552,6 +554,8 @@ static void emit_context_line(const char *reset,
struct emit_callback *ecbdata,
const char *line, int len)
{
+ ecbdata->lno_in_postimage++;
+ ecbdata->lno_in_preimage++;
emit_line_checked(reset, ecbdata, line, len,
DIFF_CONTEXT, WSEH_CONTEXT, ' ');
}
@@ -662,13 +666,10 @@ static void emit_rewrite_lines(struct emit_callback *ecb,
endp = memchr(data, '\n', size);
len = endp ? (endp - data + 1) : size;
- if (prefix != '+') {
- ecb->lno_in_preimage++;
+ if (prefix != '+')
emit_del_line(reset, ecb, data, len);
- } else {
- ecb->lno_in_postimage++;
+ else
emit_add_line(reset, ecb, data, len);
- }
size -= len;
data += len;
}
@@ -1293,16 +1294,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
switch (line[0]) {
case '+':
- ecbdata->lno_in_postimage++;
emit_add_line(reset, ecbdata, line + 1, len - 1);
break;
case '-':
- ecbdata->lno_in_preimage++;
emit_del_line(reset, ecbdata, line + 1, len - 1);
break;
case ' ':
- ecbdata->lno_in_postimage++;
- ecbdata->lno_in_preimage++;
emit_context_line(reset, ecbdata, line + 1, len - 1);
break;
default:
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 02/10] diff: emit_{add, del, context}_line to increase {pre,post}image line count
2016-09-11 5:42 ` [PATCH 02/10] diff: emit_{add, del, context}_line to increase {pre,post}image line count Stefan Beller
@ 2016-09-12 23:47 ` Junio C Hamano
0 siblings, 0 replies; 21+ messages in thread
From: Junio C Hamano @ 2016-09-12 23:47 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, Stefan Beller
Stefan Beller <stefanbeller@gmail.com> writes:
> From: Stefan Beller <sbeller@google.com>
>
> At all call sites of emit_{add, del, context}_line we increment the line
> count, so move it into the respective functions to make the code at the
> calling site a bit clearer.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> diff.c | 15 ++++++---------
> 1 file changed, 6 insertions(+), 9 deletions(-)
I am mostly in favor of this change, as the calls to these three
functions are always preceded by increment of these fields, but it
is "mostly" exactly because the reverse is not true. Namely ...
> @@ -1293,16 +1294,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
>
> switch (line[0]) {
> case '+':
> - ecbdata->lno_in_postimage++;
> emit_add_line(reset, ecbdata, line + 1, len - 1);
> break;
> case '-':
> - ecbdata->lno_in_preimage++;
> emit_del_line(reset, ecbdata, line + 1, len - 1);
> break;
> case ' ':
> - ecbdata->lno_in_postimage++;
> - ecbdata->lno_in_preimage++;
> emit_context_line(reset, ecbdata, line + 1, len - 1);
> break;
> default:
... there still needs an increment in the context lines, not shown
in the patch, just after this "default:". I think the patch is OK
as the comment after this "default:" (also not shown in the patch)
makes it clear what is going on.
Thanks.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 03/10] diff.c: drop tautologous condition in emit_line_0
2016-09-11 5:42 [PATCH 00/10] Another preparatory series for rename detection Stefan Beller
2016-09-11 5:42 ` [PATCH 01/10] diff: move line ending check into emit_hunk_header Stefan Beller
2016-09-11 5:42 ` [PATCH 02/10] diff: emit_{add, del, context}_line to increase {pre,post}image line count Stefan Beller
@ 2016-09-11 5:42 ` Stefan Beller
2016-09-12 23:53 ` Junio C Hamano
2016-09-11 5:42 ` [PATCH 04/10] diff.c: rename diff_flush_patch to diff_flush_patch_filepair Stefan Beller
` (6 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Stefan Beller @ 2016-09-11 5:42 UTC (permalink / raw)
To: gitster; +Cc: git, Stefan Beller
From: Stefan Beller <sbeller@google.com>
When `first` equals '\r', then it cannot equal '\n', which makes
`!has_trailing_newline` always true if `first` is '\r'.
Drop that check.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
diff.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/diff.c b/diff.c
index 156c2aa..9d2e704 100644
--- a/diff.c
+++ b/diff.c
@@ -460,8 +460,7 @@ static void emit_line_0(struct diff_options *o, const char *set, const char *res
if (len == 0) {
has_trailing_newline = (first == '\n');
- has_trailing_carriage_return = (!has_trailing_newline &&
- (first == '\r'));
+ has_trailing_carriage_return = (first == '\r');
nofirst = has_trailing_newline || has_trailing_carriage_return;
} else {
has_trailing_newline = (len > 0 && line[len-1] == '\n');
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 03/10] diff.c: drop tautologous condition in emit_line_0
2016-09-11 5:42 ` [PATCH 03/10] diff.c: drop tautologous condition in emit_line_0 Stefan Beller
@ 2016-09-12 23:53 ` Junio C Hamano
2016-09-16 23:04 ` Stefan Beller
0 siblings, 1 reply; 21+ messages in thread
From: Junio C Hamano @ 2016-09-12 23:53 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, Stefan Beller
Stefan Beller <stefanbeller@gmail.com> writes:
> diff --git a/diff.c b/diff.c
> index 156c2aa..9d2e704 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -460,8 +460,7 @@ static void emit_line_0(struct diff_options *o, const char *set, const char *res
>
> if (len == 0) {
> has_trailing_newline = (first == '\n');
> - has_trailing_carriage_return = (!has_trailing_newline &&
> - (first == '\r'));
> + has_trailing_carriage_return = (first == '\r');
> nofirst = has_trailing_newline || has_trailing_carriage_return;
> } else {
> has_trailing_newline = (len > 0 && line[len-1] == '\n');
Interesting.
This may be a mis-conversion at 250f7993 ("diff.c: split emit_line()
from the first char and the rest of the line", 2009-09-14), I
suspect. The original took line[] with length and peeked for '\n',
and when it saw one, it decremented length before checking
line[len-1] for '\r'.
But of course if there is only one byte on the line (i.e. len == 0
after first is stripped off), it cannot be both '\n' or '\r' at the
same time.
Thanks for spotting.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 03/10] diff.c: drop tautologous condition in emit_line_0
2016-09-12 23:53 ` Junio C Hamano
@ 2016-09-16 23:04 ` Stefan Beller
0 siblings, 0 replies; 21+ messages in thread
From: Stefan Beller @ 2016-09-16 23:04 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefan Beller, git
On Mon, Sep 12, 2016 at 4:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <stefanbeller@gmail.com> writes:
>
>> diff --git a/diff.c b/diff.c
>> index 156c2aa..9d2e704 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -460,8 +460,7 @@ static void emit_line_0(struct diff_options *o, const char *set, const char *res
>>
>> if (len == 0) {
>> has_trailing_newline = (first == '\n');
>> - has_trailing_carriage_return = (!has_trailing_newline &&
>> - (first == '\r'));
>> + has_trailing_carriage_return = (first == '\r');
>> nofirst = has_trailing_newline || has_trailing_carriage_return;
>> } else {
>> has_trailing_newline = (len > 0 && line[len-1] == '\n');
>
> Interesting.
>
> This may be a mis-conversion at 250f7993 ("diff.c: split emit_line()
> from the first char and the rest of the line", 2009-09-14), I
> suspect. The original took line[] with length and peeked for '\n',
> and when it saw one, it decremented length before checking
> line[len-1] for '\r'.
>
> But of course if there is only one byte on the line (i.e. len == 0
> after first is stripped off), it cannot be both '\n' or '\r' at the
> same time.
>
> Thanks for spotting.
Oh, right, it used to be possible to remove \r\n completely and that information
was then kept as has_trailing_newline = has_trailing_carriage_return = 1;
and the resulting line is kept completely without ending line.
After some thought I don't think I can use this mis-conversion
to trigger a bug though, because the len=0 can only ever happen
if first is '\n' alone essentially.
Another thing I noticed when playing around with diffs:
$ printf "\r\n" >crlf
$ git commit crlf -m "add file crlf, empty line"
$ printf "non zero length\r\n" >crlf
$ diff --git a/crlf b/crlf
$ index d3f5a12..ece7140 100644
--- a/crlf
+++ b/crlf
@@ -1 +1 @@
-
+non zero length^M
$ # The - line is missing a ^M ?
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 04/10] diff.c: rename diff_flush_patch to diff_flush_patch_filepair
2016-09-11 5:42 [PATCH 00/10] Another preparatory series for rename detection Stefan Beller
` (2 preceding siblings ...)
2016-09-11 5:42 ` [PATCH 03/10] diff.c: drop tautologous condition in emit_line_0 Stefan Beller
@ 2016-09-11 5:42 ` Stefan Beller
2016-09-12 23:53 ` Junio C Hamano
2016-09-11 5:42 ` [PATCH 05/10] diff.c: reintroduce diff_flush_patch for all files Stefan Beller
` (5 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Stefan Beller @ 2016-09-11 5:42 UTC (permalink / raw)
To: gitster; +Cc: git, Stefan Beller
From: Stefan Beller <sbeller@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
diff.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/diff.c b/diff.c
index 9d2e704..85fb887 100644
--- a/diff.c
+++ b/diff.c
@@ -4176,7 +4176,7 @@ int diff_unmodified_pair(struct diff_filepair *p)
return 0;
}
-static void diff_flush_patch(struct diff_filepair *p, struct diff_options *o)
+static void diff_flush_patch_filepair(struct diff_filepair *p, struct diff_options *o)
{
if (diff_unmodified_pair(p))
return;
@@ -4672,7 +4672,7 @@ void diff_flush(struct diff_options *options)
DIFF_OPT_TST(options, EXIT_WITH_STATUS) &&
DIFF_OPT_TST(options, DIFF_FROM_CONTENTS)) {
/*
- * run diff_flush_patch for the exit status. setting
+ * run diff_flush_patch_filepair for the exit status. setting
* options->file to /dev/null should be safe, because we
* aren't supposed to produce any output anyway.
*/
@@ -4685,7 +4685,7 @@ void diff_flush(struct diff_options *options)
for (i = 0; i < q->nr; i++) {
struct diff_filepair *p = q->queue[i];
if (check_pair_status(p))
- diff_flush_patch(p, options);
+ diff_flush_patch_filepair(p, options);
if (options->found_changes)
break;
}
@@ -4705,7 +4705,7 @@ void diff_flush(struct diff_options *options)
for (i = 0; i < q->nr; i++) {
struct diff_filepair *p = q->queue[i];
if (check_pair_status(p))
- diff_flush_patch(p, options);
+ diff_flush_patch_filepair(p, options);
}
}
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 05/10] diff.c: reintroduce diff_flush_patch for all files
2016-09-11 5:42 [PATCH 00/10] Another preparatory series for rename detection Stefan Beller
` (3 preceding siblings ...)
2016-09-11 5:42 ` [PATCH 04/10] diff.c: rename diff_flush_patch to diff_flush_patch_filepair Stefan Beller
@ 2016-09-11 5:42 ` Stefan Beller
2016-09-13 0:05 ` Junio C Hamano
2016-09-11 5:42 ` [PATCH 06/10] diff.c: emit_line_0 can handle no color Stefan Beller
` (4 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Stefan Beller @ 2016-09-11 5:42 UTC (permalink / raw)
To: gitster; +Cc: git, Stefan Beller
From: Stefan Beller <sbeller@google.com>
---
diff.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/diff.c b/diff.c
index 85fb887..87b1bb2 100644
--- a/diff.c
+++ b/diff.c
@@ -4608,6 +4608,17 @@ void diff_warn_rename_limit(const char *varname, int needed, int degraded_cc)
warning(rename_limit_advice, varname, needed);
}
+static void diff_flush_patch(struct diff_options *o)
+{
+ int i;
+ struct diff_queue_struct *q = &diff_queued_diff;
+ for (i = 0; i < q->nr; i++) {
+ struct diff_filepair *p = q->queue[i];
+ if (check_pair_status(p))
+ diff_flush_patch_filepair(p, o);
+ }
+}
+
void diff_flush(struct diff_options *options)
{
struct diff_queue_struct *q = &diff_queued_diff;
@@ -4702,11 +4713,7 @@ void diff_flush(struct diff_options *options)
}
}
- for (i = 0; i < q->nr; i++) {
- struct diff_filepair *p = q->queue[i];
- if (check_pair_status(p))
- diff_flush_patch_filepair(p, options);
- }
+ diff_flush_patch(options);
}
if (output_format & DIFF_FORMAT_CALLBACK)
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 05/10] diff.c: reintroduce diff_flush_patch for all files
2016-09-11 5:42 ` [PATCH 05/10] diff.c: reintroduce diff_flush_patch for all files Stefan Beller
@ 2016-09-13 0:05 ` Junio C Hamano
0 siblings, 0 replies; 21+ messages in thread
From: Junio C Hamano @ 2016-09-13 0:05 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, Stefan Beller
Stefan Beller <stefanbeller@gmail.com> writes:
> From: Stefan Beller <sbeller@google.com>
>
> ---
This shows why 04/10 should have had the overall plan for these two
steps. We want a short-and-sweet name "diff-flush-patch" to mean
"flush all the queued diff elements" so rename the singleton one
from diff-flush-patch to diff-flush-patch-filepair to make room in
04/10 and then introduce the "diff-flush-patch-all" in 05/10.
I just said with the above explanation the changes in 04/10 and
05/10 become undertandable, which is a bit different from being
justifiable. "diff_flush_raw()", "diff_flush_stat()", etc. are
_all_ about a single filepair. I'd rather see diff_flush_patch()
to be also about a single filepair.
It may be helpful to have a helper that calls diff_flush_patch() for
all filepairs in the queue, but can't that function get a new name
instead? By definition, it will be called much less often than a
pair-wise one, so it can afford to have a longer name.
diff_flush_patch_queue() or something, perhaps? I am not sure if it
should be diff_flush_queue_patch(), or even
diff_flush_queue(struct diff_options *o, diff_flush_fn fn);
where diff_flush_fn is
typedef void (*diff_flush_fn)(struct diff_filepair *p,
struct diff_options *o, void *other_data)
that can be used to flush the queue by calling any of these
filepair-wise flush functions like diff_flush_{raw,stat,checkdiff,patch}.
This last approach might be overkill, but if you want to try it,
you'd need a preparatory step to give an unused "void *other" to
diff_flush_{raw,patch,checkdiff} as diff_flush_stat() is an oddball
that needs an extra "accumulator" pointer. I actually wonder if
that "diffstat" pointer should become part of "struct diff_options",
though. Anyway.
> diff.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index 85fb887..87b1bb2 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -4608,6 +4608,17 @@ void diff_warn_rename_limit(const char *varname, int needed, int degraded_cc)
> warning(rename_limit_advice, varname, needed);
> }
>
> +static void diff_flush_patch(struct diff_options *o)
> +{
> + int i;
> + struct diff_queue_struct *q = &diff_queued_diff;
> + for (i = 0; i < q->nr; i++) {
> + struct diff_filepair *p = q->queue[i];
> + if (check_pair_status(p))
> + diff_flush_patch_filepair(p, o);
> + }
> +}
> +
> void diff_flush(struct diff_options *options)
> {
> struct diff_queue_struct *q = &diff_queued_diff;
> @@ -4702,11 +4713,7 @@ void diff_flush(struct diff_options *options)
> }
> }
>
> - for (i = 0; i < q->nr; i++) {
> - struct diff_filepair *p = q->queue[i];
> - if (check_pair_status(p))
> - diff_flush_patch_filepair(p, options);
> - }
> + diff_flush_patch(options);
> }
>
> if (output_format & DIFF_FORMAT_CALLBACK)
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 06/10] diff.c: emit_line_0 can handle no color
2016-09-11 5:42 [PATCH 00/10] Another preparatory series for rename detection Stefan Beller
` (4 preceding siblings ...)
2016-09-11 5:42 ` [PATCH 05/10] diff.c: reintroduce diff_flush_patch for all files Stefan Beller
@ 2016-09-11 5:42 ` Stefan Beller
2016-09-13 0:11 ` Junio C Hamano
2016-09-11 5:42 ` [PATCH 07/10] diff.c: convert fn_out_consume to use emit_line_* Stefan Beller
` (3 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Stefan Beller @ 2016-09-11 5:42 UTC (permalink / raw)
To: gitster; +Cc: git, Stefan Beller
From: Stefan Beller <sbeller@google.com>
---
diff.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/diff.c b/diff.c
index 87b1bb2..2aefd0f 100644
--- a/diff.c
+++ b/diff.c
@@ -473,11 +473,13 @@ static void emit_line_0(struct diff_options *o, const char *set, const char *res
}
if (len || !nofirst) {
- fputs(set, file);
+ if (set)
+ fputs(set, file);
if (!nofirst)
fputc(first, file);
fwrite(line, len, 1, file);
- fputs(reset, file);
+ if (reset)
+ fputs(reset, file);
}
if (has_trailing_carriage_return)
fputc('\r', file);
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 06/10] diff.c: emit_line_0 can handle no color
2016-09-11 5:42 ` [PATCH 06/10] diff.c: emit_line_0 can handle no color Stefan Beller
@ 2016-09-13 0:11 ` Junio C Hamano
2016-09-13 0:25 ` Stefan Beller
0 siblings, 1 reply; 21+ messages in thread
From: Junio C Hamano @ 2016-09-13 0:11 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, Stefan Beller
Stefan Beller <stefanbeller@gmail.com> writes:
> From: Stefan Beller <sbeller@google.com>
>
> ---
"X can do Y" can be taken as a statement of fact (to which "so
what?" is an appropriate response), a desire (to which "then please
say 'make X do Y' instead" is an appropriate response), or a report
of a bug (to which "please explain why X should be forbidden from
doing Y" is an appropriate response).
This is way under-explained. I think this is "make X do Y" kind,
and if so, please say so and possibly why it is a good idea to teach
X how to do Y.
Thanks.
> diff.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index 87b1bb2..2aefd0f 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -473,11 +473,13 @@ static void emit_line_0(struct diff_options *o, const char *set, const char *res
> }
>
> if (len || !nofirst) {
> - fputs(set, file);
> + if (set)
> + fputs(set, file);
> if (!nofirst)
> fputc(first, file);
> fwrite(line, len, 1, file);
> - fputs(reset, file);
> + if (reset)
> + fputs(reset, file);
> }
> if (has_trailing_carriage_return)
> fputc('\r', file);
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 06/10] diff.c: emit_line_0 can handle no color
2016-09-13 0:11 ` Junio C Hamano
@ 2016-09-13 0:25 ` Stefan Beller
0 siblings, 0 replies; 21+ messages in thread
From: Stefan Beller @ 2016-09-13 0:25 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefan Beller, git
On Mon, Sep 12, 2016 at 5:11 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <stefanbeller@gmail.com> writes:
>
>> From: Stefan Beller <sbeller@google.com>
>>
>> ---
>
> "X can do Y" can be taken as a statement of fact (to which "so
> what?" is an appropriate response), a desire (to which "then please
> say 'make X do Y' instead" is an appropriate response), or a report
> of a bug (to which "please explain why X should be forbidden from
> doing Y" is an appropriate response).
>
> This is way under-explained. I think this is "make X do Y" kind,
> and if so, please say so and possibly why it is a good idea to teach
> X how to do Y.
>
> Thanks.
Ok, I see the general pattern of your answers: Add more explanations.
Answering for
patch 01/10 as well as this one here:
I want to propose an option to detect&color moved lines in a patch,
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 a
line pair that got moved in the first pass. So I aim for
* collecting all output into a buffer as a first pass,
* as the second pass output the buffer.
So in a later patch I will split up the emit_line_* machinery to either
emitting to options->file or buffering if we do the 2 pass thing.
To make sure the 2 passes work correctly, we need to make sure all output
is routed through the emit_line functions, and there will be no direct writes.
Now that we will be using the emit_lines functions for non colored
output as well,
we want to pass in "no color" which I think is best done via NULL and then not
calling the output of the color writes.
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 07/10] diff.c: convert fn_out_consume to use emit_line_*
2016-09-11 5:42 [PATCH 00/10] Another preparatory series for rename detection Stefan Beller
` (5 preceding siblings ...)
2016-09-11 5:42 ` [PATCH 06/10] diff.c: emit_line_0 can handle no color Stefan Beller
@ 2016-09-11 5:42 ` Stefan Beller
2016-09-13 0:25 ` Junio C Hamano
2016-09-11 5:42 ` [PATCH 08/10] diff.c: convert emit_rewrite_diff " Stefan Beller
` (2 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: Stefan Beller @ 2016-09-11 5:42 UTC (permalink / raw)
To: gitster; +Cc: git, Stefan Beller, Stefan Beller
From: Stefan Beller <sbeller@google.com>
In the coming patches we want to eliminate writes that do not go
though emit_line_* as we later want to hook into the emit_line_*
functions to add a buffered output. To make the conversion easy,
add a function that takes a format string.
Signed-off-by: Stefan Beller <stefanbeller@gmail.com>
---
diff.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/diff.c b/diff.c
index 2aefd0f..7dcef73 100644
--- a/diff.c
+++ b/diff.c
@@ -493,6 +493,19 @@ 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,
+ const char *set, const char *reset,
+ const char *fmt, ...)
+{
+ struct strbuf sb = STRBUF_INIT;
+ va_list ap;
+ va_start(ap, fmt);
+ strbuf_vaddf(&sb, fmt, ap);
+ va_end(ap);
+
+ emit_line(o, set, reset, sb.buf, sb.len);
+}
+
static int new_blank_line_at_eof(struct emit_callback *ecbdata, const char *line, int len)
{
if (!((ecbdata->ws_rule & WS_BLANK_AT_EOF) &&
@@ -1217,7 +1230,6 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
const char *context = diff_get_color(ecbdata->color_diff, DIFF_CONTEXT);
const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);
struct diff_options *o = ecbdata->opt;
- const char *line_prefix = diff_line_prefix(o);
o->found_changes = 1;
@@ -1233,10 +1245,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
name_a_tab = strchr(ecbdata->label_path[0], ' ') ? "\t" : "";
name_b_tab = strchr(ecbdata->label_path[1], ' ') ? "\t" : "";
- fprintf(o->file, "%s%s--- %s%s%s\n",
- line_prefix, meta, ecbdata->label_path[0], reset, name_a_tab);
- fprintf(o->file, "%s%s+++ %s%s%s\n",
- line_prefix, meta, ecbdata->label_path[1], reset, name_b_tab);
+ emit_line_fmt(o, meta, reset, "--- %s%s\n",
+ ecbdata->label_path[0], name_a_tab);
+
+ emit_line_fmt(o, meta, reset, "+++ %s%s\n",
+ ecbdata->label_path[1], name_b_tab);
+
ecbdata->label_path[0] = ecbdata->label_path[1] = NULL;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH 07/10] diff.c: convert fn_out_consume to use emit_line_*
2016-09-11 5:42 ` [PATCH 07/10] diff.c: convert fn_out_consume to use emit_line_* Stefan Beller
@ 2016-09-13 0:25 ` Junio C Hamano
2016-09-13 0:41 ` Stefan Beller
0 siblings, 1 reply; 21+ messages in thread
From: Junio C Hamano @ 2016-09-13 0:25 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, Stefan Beller
Stefan Beller <stefanbeller@gmail.com> writes:
> diff --git a/diff.c b/diff.c
> index 2aefd0f..7dcef73 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -493,6 +493,19 @@ 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,
> + const char *set, const char *reset,
> + const char *fmt, ...)
> +{
> + struct strbuf sb = STRBUF_INIT;
> + va_list ap;
> + va_start(ap, fmt);
> + strbuf_vaddf(&sb, fmt, ap);
> + va_end(ap);
> +
> + emit_line(o, set, reset, sb.buf, sb.len);
> +}
> +
> static int new_blank_line_at_eof(struct emit_callback *ecbdata, const char *line, int len)
> {
> if (!((ecbdata->ws_rule & WS_BLANK_AT_EOF) &&
> @@ -1217,7 +1230,6 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
> const char *context = diff_get_color(ecbdata->color_diff, DIFF_CONTEXT);
> const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);
> struct diff_options *o = ecbdata->opt;
> - const char *line_prefix = diff_line_prefix(o);
>
> o->found_changes = 1;
>
> @@ -1233,10 +1245,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
> name_a_tab = strchr(ecbdata->label_path[0], ' ') ? "\t" : "";
> name_b_tab = strchr(ecbdata->label_path[1], ' ') ? "\t" : "";
>
> - fprintf(o->file, "%s%s--- %s%s%s\n",
> - line_prefix, meta, ecbdata->label_path[0], reset, name_a_tab);
> - fprintf(o->file, "%s%s+++ %s%s%s\n",
> - line_prefix, meta, ecbdata->label_path[1], reset, name_b_tab);
> + emit_line_fmt(o, meta, reset, "--- %s%s\n",
> + ecbdata->label_path[0], name_a_tab);
> +
> + emit_line_fmt(o, meta, reset, "+++ %s%s\n",
> + ecbdata->label_path[1], name_b_tab);
Hmph, the original showed the following for the name-a line:
diff_line_prefix(o) META "--- " label_path RESET name_a_tab LF
The updated one calls emit_line_fmt() with o, meta, reset, fmt and
args, and then
* strbuf_vaddf(&buf, "--- %s%s\n", label_path, name_a_tab) creates
a string "--- " + label_path + LF
* emit_line() is called on the whole thing with META and RESET
* which is emit_line_0() that encloses the whole thing between META
and RESET but knows the trailing LF should come after RESET.
So the coloring seems to be correct, but I am not sure where the
line-prefix went.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH 07/10] diff.c: convert fn_out_consume to use emit_line_*
2016-09-13 0:25 ` Junio C Hamano
@ 2016-09-13 0:41 ` Stefan Beller
0 siblings, 0 replies; 21+ messages in thread
From: Stefan Beller @ 2016-09-13 0:41 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Stefan Beller, git
On Mon, Sep 12, 2016 at 5:25 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <stefanbeller@gmail.com> writes:
>
>> diff --git a/diff.c b/diff.c
>> index 2aefd0f..7dcef73 100644
>> --- a/diff.c
>> +++ b/diff.c
>> @@ -493,6 +493,19 @@ 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,
>> + const char *set, const char *reset,
>> + const char *fmt, ...)
>> +{
>> + struct strbuf sb = STRBUF_INIT;
>> + va_list ap;
>> + va_start(ap, fmt);
>> + strbuf_vaddf(&sb, fmt, ap);
>> + va_end(ap);
>> +
>> + emit_line(o, set, reset, sb.buf, sb.len);
>> +}
>> +
>> static int new_blank_line_at_eof(struct emit_callback *ecbdata, const char *line, int len)
>> {
>> if (!((ecbdata->ws_rule & WS_BLANK_AT_EOF) &&
>> @@ -1217,7 +1230,6 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
>> const char *context = diff_get_color(ecbdata->color_diff, DIFF_CONTEXT);
>> const char *reset = diff_get_color(ecbdata->color_diff, DIFF_RESET);
>> struct diff_options *o = ecbdata->opt;
>> - const char *line_prefix = diff_line_prefix(o);
>>
>> o->found_changes = 1;
>>
>> @@ -1233,10 +1245,12 @@ static void fn_out_consume(void *priv, char *line, unsigned long len)
>> name_a_tab = strchr(ecbdata->label_path[0], ' ') ? "\t" : "";
>> name_b_tab = strchr(ecbdata->label_path[1], ' ') ? "\t" : "";
>>
>> - fprintf(o->file, "%s%s--- %s%s%s\n",
>> - line_prefix, meta, ecbdata->label_path[0], reset, name_a_tab);
>> - fprintf(o->file, "%s%s+++ %s%s%s\n",
>> - line_prefix, meta, ecbdata->label_path[1], reset, name_b_tab);
>> + emit_line_fmt(o, meta, reset, "--- %s%s\n",
>> + ecbdata->label_path[0], name_a_tab);
>> +
>> + emit_line_fmt(o, meta, reset, "+++ %s%s\n",
>> + ecbdata->label_path[1], name_b_tab);
>
> Hmph, the original showed the following for the name-a line:
>
> diff_line_prefix(o) META "--- " label_path RESET name_a_tab LF
>
> The updated one calls emit_line_fmt() with o, meta, reset, fmt and
> args, and then
>
> * strbuf_vaddf(&buf, "--- %s%s\n", label_path, name_a_tab) creates
> a string "--- " + label_path + LF
>
> * emit_line() is called on the whole thing with META and RESET
Oh right, that is a real issue, i.e. name_b_tab is not colored. I'll
have to think about that.
>
> * which is emit_line_0() that encloses the whole thing between META
> and RESET but knows the trailing LF should come after RESET.
>
> So the coloring seems to be correct, but I am not sure where the
> line-prefix went.
emit_line_0 puts the line_prefix by default in front, i.e.
it emits
line_prefix (SET) (first char) line (RESET) (CR/LF)
with things in parenthesis optional.
As said in 6/10 I think the emit_line_0 function is a great starting point
to build up a buffer when we do a two passes output. For that I want to channel
all output through emit_line_*.
---
I am done converting all diff output to emit_line for a rought first series,
and now all tests pass when asking for a buffered output.
So I started building the --color-moved on top of that, which seems
to work as well. The diff stat is ~700 insertions and ~300 deletions
compared to 2.10, however we call out to the xdl stuff only once.
Comparing that to the original quick-hack-patch
https://public-inbox.org/git/20160906070151.15163-1-stefanbeller@gmail.com/
which has just 280 additions, 10 deletions;
we seem to need about ~350 refactor lines and
slightly slower emissions in the non-color-moved case
(all the calls to emit_line instead of directly using fprintf/fputs/fputc)
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 08/10] diff.c: convert emit_rewrite_diff to use emit_line_*
2016-09-11 5:42 [PATCH 00/10] Another preparatory series for rename detection Stefan Beller
` (6 preceding siblings ...)
2016-09-11 5:42 ` [PATCH 07/10] diff.c: convert fn_out_consume to use emit_line_* Stefan Beller
@ 2016-09-11 5:42 ` Stefan Beller
2016-09-11 5:42 ` [PATCH 09/10] diff.c: convert emit_rewrite_lines to use emit_line_0 Stefan Beller
2016-09-11 5:42 ` [PATCH 10/10] submodule.c: convert show_submodule_summary to use emit_line_fmt Stefan Beller
9 siblings, 0 replies; 21+ messages in thread
From: Stefan Beller @ 2016-09-11 5:42 UTC (permalink / raw)
To: gitster; +Cc: git, Stefan Beller
From: Stefan Beller <sbeller@google.com>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
diff.c | 33 ++++++++++++++++++---------------
1 file changed, 18 insertions(+), 15 deletions(-)
diff --git a/diff.c b/diff.c
index 7dcef73..98d6655 100644
--- a/diff.c
+++ b/diff.c
@@ -653,17 +653,17 @@ static void remove_tempfile(void)
}
}
-static void print_line_count(FILE *file, int count)
+static void add_line_count(struct strbuf *out, int count)
{
switch (count) {
case 0:
- fprintf(file, "0,0");
+ strbuf_addstr(out, "0,0");
break;
case 1:
- fprintf(file, "1");
+ strbuf_addstr(out, "1");
break;
default:
- fprintf(file, "1,%d", count);
+ strbuf_addf(out, "1,%d", count);
break;
}
}
@@ -714,7 +714,7 @@ static void emit_rewrite_diff(const char *name_a,
char *data_one, *data_two;
size_t size_one, size_two;
struct emit_callback ecbdata;
- const char *line_prefix = diff_line_prefix(o);
+ struct strbuf out = STRBUF_INIT;
if (diff_mnemonic_prefix && DIFF_OPT_TST(o, REVERSE_DIFF)) {
a_prefix = o->b_prefix;
@@ -752,20 +752,23 @@ static void emit_rewrite_diff(const char *name_a,
ecbdata.lno_in_preimage = 1;
ecbdata.lno_in_postimage = 1;
+ emit_line_fmt(o, metainfo, reset, "--- %s%s\n", a_name.buf, name_a_tab);
+ emit_line_fmt(o, metainfo, reset, "+++ %s%s\n", b_name.buf, name_b_tab);
+
lc_a = count_lines(data_one, size_one);
lc_b = count_lines(data_two, size_two);
- fprintf(o->file,
- "%s%s--- %s%s%s\n%s%s+++ %s%s%s\n%s%s@@ -",
- line_prefix, metainfo, a_name.buf, name_a_tab, reset,
- line_prefix, metainfo, b_name.buf, name_b_tab, reset,
- line_prefix, fraginfo);
+
+ strbuf_addstr(&out, "@@ -");
if (!o->irreversible_delete)
- print_line_count(o->file, lc_a);
+ add_line_count(&out, lc_a);
else
- fprintf(o->file, "?,?");
- fprintf(o->file, " +");
- print_line_count(o->file, lc_b);
- fprintf(o->file, " @@%s\n", reset);
+ strbuf_addstr(&out, "?,?");
+ strbuf_addstr(&out, " +");
+ add_line_count(&out, lc_b);
+ strbuf_addstr(&out, " @@\n");
+ emit_line(o, fraginfo, reset, out.buf, out.len);
+ strbuf_release(&out);
+
if (lc_a && !o->irreversible_delete)
emit_rewrite_lines(&ecbdata, '-', data_one, size_one);
if (lc_b)
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 09/10] diff.c: convert emit_rewrite_lines to use emit_line_0
2016-09-11 5:42 [PATCH 00/10] Another preparatory series for rename detection Stefan Beller
` (7 preceding siblings ...)
2016-09-11 5:42 ` [PATCH 08/10] diff.c: convert emit_rewrite_diff " Stefan Beller
@ 2016-09-11 5:42 ` Stefan Beller
2016-09-11 5:42 ` [PATCH 10/10] submodule.c: convert show_submodule_summary to use emit_line_fmt Stefan Beller
9 siblings, 0 replies; 21+ messages in thread
From: Stefan Beller @ 2016-09-11 5:42 UTC (permalink / raw)
To: gitster; +Cc: git, Stefan Beller
From: Stefan Beller <sbeller@google.com>
---
diff.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/diff.c b/diff.c
index 98d6655..255fbf3 100644
--- a/diff.c
+++ b/diff.c
@@ -690,7 +690,7 @@ static void emit_rewrite_lines(struct emit_callback *ecb,
if (!endp) {
const char *context = diff_get_color(ecb->color_diff,
DIFF_CONTEXT);
- putc('\n', ecb->opt->file);
+ emit_line_0(ecb->opt, 0, 0, 0, "\n", 1);
emit_line_0(ecb->opt, context, reset, '\\',
nneof, strlen(nneof));
}
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH 10/10] submodule.c: convert show_submodule_summary to use emit_line_fmt
2016-09-11 5:42 [PATCH 00/10] Another preparatory series for rename detection Stefan Beller
` (8 preceding siblings ...)
2016-09-11 5:42 ` [PATCH 09/10] diff.c: convert emit_rewrite_lines to use emit_line_0 Stefan Beller
@ 2016-09-11 5:42 ` Stefan Beller
9 siblings, 0 replies; 21+ messages in thread
From: Stefan Beller @ 2016-09-11 5:42 UTC (permalink / raw)
To: gitster; +Cc: git, Stefan Beller
From: Stefan Beller <sbeller@google.com>
---
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 255fbf3..c00306b 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, ...)
{
@@ -2310,8 +2310,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)) {
strbuf_release(&sb);
return;
}
- strbuf_addf(&sb, "%s%sSubmodule %s %s..", line_prefix, meta, path,
- find_unique_abbrev(one, DEFAULT_ABBREV));
+ strbuf_addf(&sb, "Submodule %s %s..", path,
+ find_unique_abbrev(one, DEFAULT_ABBREV));
if (!fast_backward && !fast_forward)
strbuf_addch(&sb, '.');
strbuf_addf(&sb, "%s", find_unique_abbrev(two, DEFAULT_ABBREV));
if (message)
- strbuf_addf(&sb, " %s%s\n", message, reset);
+ strbuf_addf(&sb, " %s\n", message);
else
- strbuf_addf(&sb, "%s:%s\n", fast_backward ? " (rewind)" : "", reset);
- fwrite(sb.buf, sb.len, 1, f);
+ strbuf_addf(&sb, "%s:\n", fast_backward ? " (rewind)" : "");
+ emit_line_fmt(o, meta, reset, "%s", sb.buf);
if (!message) /* only NULL if we succeeded in setting up the walk */
- print_submodule_summary(&rev, f, line_prefix, del, add, reset);
+ print_submodule_summary(&rev, o, del, add, reset);
if (left)
clear_commit_marks(left, ~0);
if (right)
diff --git a/submodule.h b/submodule.h
index 2af9390..fefc0ab 100644
--- a/submodule.h
+++ b/submodule.h
@@ -41,8 +41,7 @@ int parse_submodule_update_strategy(const char *value,
struct submodule_update_strategy *dst);
const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);
void handle_ignore_submodules_arg(struct diff_options *diffopt, const char *);
-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);
--
2.7.4
^ permalink raw reply related [flat|nested] 21+ messages in thread