git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Stefan Beller <sbeller@google.com>
To: gitster@pobox.com, peff@peff.net, chriscool@tuxfamily.org
Cc: git@vger.kernel.org, Stefan Beller <sbeller@google.com>
Subject: [RFC/PATCH 16/17] diff: buffer output in emit_line_0
Date: Mon, 12 Sep 2016 21:46:12 -0700	[thread overview]
Message-ID: <20160913044613.1037-17-sbeller@google.com> (raw)
In-Reply-To: <20160913044613.1037-1-sbeller@google.com>

emit_line_0 factors out the emission part into emit_line_emission,
and depending on the diff_options->use_buffer the emission
will be performed directly when calling emit_line_0 or after the
whole process is done, i.e. by buffering we have add the possibility
for a second pass over the whole output before doing the actual
output.

In 6440d34 (2012-03-14, diff: tweak a _copy_ of diff_options with
word-diff) we introduced a duplicate diff options struct for word
emissions as we may have different regex settings in there.

When buffering the output, we need to operate on just one buffer,
so we have to copy back the emissions of the word buffer into the main
buffer.

Unconditionally enable output via buffer in this patch as it yields
a great opportunity for testing, i.e. all the diff tests from the
test suite pass without having reordering issues (i.e. only parts
of the output got buffered, and we forgot to buffer other parts).
The test suite passes, which gives confidence that we converted all
functions to use emit_line_* for output.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 diff.c | 151 +++++++++++++++++++++++++++++++++++++++++++++++++++++------------
 diff.h |  18 ++++++++
 2 files changed, 142 insertions(+), 27 deletions(-)

diff --git a/diff.c b/diff.c
index 68da135..e240e89 100644
--- a/diff.c
+++ b/diff.c
@@ -449,42 +449,86 @@ static void check_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2,
 	ecbdata->blank_at_eof_in_postimage = (at - l2) + 1;
 }
 
-static void emit_line_0(struct diff_options *o, const char *set, const char *reset,
-			int first, const char *line, int len)
+static void emit_line_emission(struct diff_options *o, struct line_emission *e)
 {
-	int has_trailing_newline, has_trailing_carriage_return;
-	int nofirst;
 	FILE *file = o->file;
 
 	fputs(diff_line_prefix(o), file);
 
+	if (e->len || e->first) {
+		if (e->set)
+			fputs(e->set, file);
+		if (e->first)
+			fputc(e->first, file);
+		if (e->whitespace_check) {
+			if (e->reset)
+				fputs(e->reset, file);
+			ws_check_emit(e->line, e->len, e->ws_rule,
+				      file, e->set, e->reset, e->ws);
+		} else {
+			fwrite(e->line, e->len, 1, file);
+			if (e->reset)
+				fputs(e->reset, file);
+		}
+	}
+	if (e->has_trailing_carriage_return)
+		fputc('\r', file);
+	if (e->has_trailing_newline)
+		fputc('\n', file);
+}
+
+static void free_line_emission(struct line_emission *e)
+{
+	/*
+	 * No need to free set, reset, ws as they always point to the
+	 * diff_colors[] static array. We don't own that memory.
+	 */
+	free((char*)e->line);
+}
+
+static void append_line_emission_to_buffer(struct diff_options *o,
+			     struct line_emission *e)
+{
+	struct line_emission *f;
+	ALLOC_GROW(o->line_buffer,
+		   o->line_buffer_nr + 1,
+		   o->line_buffer_alloc);
+	f = &o->line_buffer[o->line_buffer_nr++];
+	memcpy(f, e, sizeof(*e));
+
+	/* duplicate the line for now as it is not a stable pointer */
+	f->line = xmemdupz(e->line, e->len);
+}
+
+static void emit_line_0(struct diff_options *o, const char *set,
+			const char *reset, int first, const char *line, int len)
+{
+	int nofirst;
+	struct line_emission e = LINE_EMISSION_INIT;
+
 	if (len == 0) {
-		has_trailing_newline = (first == '\n');
-		has_trailing_carriage_return = (first == '\r');
-		nofirst = has_trailing_newline || has_trailing_carriage_return;
+		e.has_trailing_newline = (first == '\n');
+		e.has_trailing_carriage_return = (first == '\r');
+		nofirst = e.has_trailing_newline || e.has_trailing_carriage_return;
 	} else {
-		has_trailing_newline = (len > 0 && line[len-1] == '\n');
-		if (has_trailing_newline)
+		e.has_trailing_newline = (len > 0 && line[len-1] == '\n');
+		if (e.has_trailing_newline)
 			len--;
-		has_trailing_carriage_return = (len > 0 && line[len-1] == '\r');
-		if (has_trailing_carriage_return)
+		e.has_trailing_carriage_return = (len > 0 && line[len-1] == '\r');
+		if (e.has_trailing_carriage_return)
 			len--;
 		nofirst = 0;
 	}
 
-	if (len || !nofirst) {
-		if (set)
-			fputs(set, file);
-		if (!nofirst)
-			fputc(first, file);
-		fwrite(line, len, 1, file);
-		if (reset)
-			fputs(reset, file);
-	}
-	if (has_trailing_carriage_return)
-		fputc('\r', file);
-	if (has_trailing_newline)
-		fputc('\n', file);
+	e.set = set;
+	e.first = !nofirst ? first : 0;
+	e.line = line;
+	e.len = len;
+	e.reset = reset;
+	if (!o->use_buffer)
+		emit_line_emission(o, &e);
+	else
+		append_line_emission_to_buffer(o, &e);
 }
 
 static void emit_line(struct diff_options *o, const char *set, const char *reset,
@@ -540,9 +584,22 @@ static void emit_line_checked(const char *reset,
 		emit_line_0(ecbdata->opt, ws, reset, sign, line, len);
 	else {
 		/* Emit just the prefix, then the rest. */
-		emit_line_0(ecbdata->opt, set, reset, sign, "", 0);
-		ws_check_emit(line, len, ecbdata->ws_rule,
-			      ecbdata->opt->file, set, reset, ws);
+		if (ecbdata->opt->use_buffer) {
+			struct line_emission e;
+			e.line = line;
+			e.len = len;
+			e.ws_rule = ecbdata->ws_rule;
+			e.set = set;
+			e.reset = reset;
+			e.ws = ws;
+			e.whitespace_check = 1;
+			e.first = sign;
+			append_line_emission_to_buffer(ecbdata->opt, &e);
+		} else {
+			emit_line_0(ecbdata->opt, set, reset, sign, "", 0);
+			ws_check_emit(line, len, ecbdata->ws_rule,
+				      ecbdata->opt->file, set, reset, ws);
+		}
 	}
 }
 
@@ -1093,6 +1150,22 @@ static void diff_words_flush(struct emit_callback *ecbdata)
 	if (ecbdata->diff_words->minus.text.size ||
 	    ecbdata->diff_words->plus.text.size)
 		diff_words_show(ecbdata->diff_words);
+
+	if (ecbdata->diff_words->opt->line_buffer_nr) {
+		int i;
+		for (i = 0; i < ecbdata->diff_words->opt->line_buffer_nr; i++) {
+			struct line_emission *e =
+				&ecbdata->diff_words->opt->line_buffer[i];
+			ALLOC_GROW(ecbdata->opt->line_buffer,
+				   ecbdata->opt->line_buffer_nr + 1,
+				   ecbdata->opt->line_buffer_alloc);
+			memcpy(&ecbdata->opt->line_buffer
+					[ecbdata->opt->line_buffer_nr],
+				e, sizeof(*e));
+			ecbdata->opt->line_buffer_nr++;
+		}
+		ecbdata->diff_words->opt->line_buffer_nr = 0;
+	}
 }
 
 static void diff_filespec_load_driver(struct diff_filespec *one)
@@ -1128,6 +1201,12 @@ static void init_diff_words_data(struct emit_callback *ecbdata,
 		xcalloc(1, sizeof(struct diff_words_data));
 	ecbdata->diff_words->type = o->word_diff;
 	ecbdata->diff_words->opt = o;
+
+	/* Create our own buffer if needed. */
+	o->line_buffer = NULL;
+	o->line_buffer_nr = 0;
+	o->line_buffer_alloc = 0;
+
 	if (!o->word_regex)
 		o->word_regex = userdiff_word_regex(one);
 	if (!o->word_regex)
@@ -4649,11 +4728,29 @@ static void diff_flush_patch_all_file_pairs(struct diff_options *o)
 {
 	int i;
 	struct diff_queue_struct *q = &diff_queued_diff;
+	/*
+	 * TODO:
+	 * For testing purposes we want to make sure the diff machinery
+	 * works with the buffer. If there is anything emitted outside the
+	 * emit_line_emission, then the order is screwed up and the tests
+	 * will fail.
+	 *
+	 * We'll unset this flag in a later patch.
+	 */
+	o->use_buffer = 1;
 	for (i = 0; i < q->nr; i++) {
 		struct diff_filepair *p = q->queue[i];
 		if (check_pair_status(p))
 			diff_flush_patch(p, o);
 	}
+
+	if (o->use_buffer) {
+		for (i = 0; i < o->line_buffer_nr; i++) {
+			emit_line_emission(o, &o->line_buffer[i]);
+			free_line_emission(&o->line_buffer[i]);
+		}
+		o->line_buffer_nr = 0;
+	}
 }
 
 void diff_flush(struct diff_options *options)
diff --git a/diff.h b/diff.h
index cc5d038..4df6aa5 100644
--- a/diff.h
+++ b/diff.h
@@ -110,6 +110,20 @@ enum diff_words_type {
 	DIFF_WORDS_COLOR
 };
 
+struct line_emission {
+	const char *set;
+	const char *line;
+	const char *ws;
+	const char *reset;
+	int first;
+	int len;
+	int whitespace_check;
+	unsigned ws_rule;
+	int has_trailing_carriage_return;
+	int has_trailing_newline;
+};
+#define LINE_EMISSION_INIT {NULL, NULL, NULL, NULL, 0, 0, 0, 0, 0, 0 }
+
 struct diff_options {
 	const char *orderfile;
 	const char *pickaxe;
@@ -178,6 +192,10 @@ struct diff_options {
 	void *output_prefix_data;
 
 	int diff_path_counter;
+
+	int use_buffer;
+	struct line_emission *line_buffer;
+	int line_buffer_nr, line_buffer_alloc;
 };
 
 void emit_line_fmt(struct diff_options *o, const char *set, const char *reset,
-- 
2.10.0.21.g1da280f.dirty


  parent reply	other threads:[~2016-09-13  4:47 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-13  4:45 [RFC/PATCH 00/17] Stefan Beller
2016-09-13  4:45 ` [RFC/PATCH 01/17] diff: move line ending check into emit_hunk_header Stefan Beller
2016-09-13 14:42   ` René Scharfe
2016-09-13 22:40     ` Stefan Beller
2016-09-13  4:45 ` [RFC/PATCH 02/17] diff: emit_{add, del, context}_line to increase {pre,post}image line count Stefan Beller
2016-09-13  4:45 ` [RFC/PATCH 03/17] diff.c: drop tautologous condition in emit_line_0 Stefan Beller
2016-09-13  4:46 ` [RFC/PATCH 04/17] diff.c: factor out diff_flush_patch_all_file_pairs Stefan Beller
2016-09-13  4:46 ` [RFC/PATCH 05/17] diff.c: emit_line_0 can handle no color setting Stefan Beller
2016-09-13 22:51   ` Junio C Hamano
2016-09-13  4:46 ` [RFC/PATCH 06/17] diff.c: convert fn_out_consume to use emit_line_* Stefan Beller
2016-09-13 22:56   ` Junio C Hamano
2016-09-13  4:46 ` [RFC/PATCH 07/17] diff.c: convert builtin_diff " Stefan Beller
2016-09-13  4:46 ` [RFC/PATCH 08/17] diff.c: convert emit_rewrite_diff " Stefan Beller
2016-09-13  4:46 ` [RFC/PATCH 09/17] diff.c: convert emit_rewrite_lines " Stefan Beller
2016-09-13  4:46 ` [RFC/PATCH 10/17] submodule.c: convert show_submodule_summary to use emit_line_fmt Stefan Beller
2016-09-13 23:02   ` Junio C Hamano
2016-09-13 23:09     ` Stefan Beller
2016-09-13  4:46 ` [RFC/PATCH 11/17] diff.c: convert emit_binary_diff_body to use emit_line_* Stefan Beller
2016-09-13  4:46 ` [RFC/PATCH 12/17] diff.c: convert show_stats " Stefan Beller
2016-09-13  4:46 ` [RFC/PATCH 13/17] diff.c: convert word diffing " Stefan Beller
2016-09-13  4:46 ` [RFC/PATCH 14/17] diff.c: convert diff_flush " Stefan Beller
2016-09-13  4:46 ` [RFC/PATCH 15/17] diff.c: convert diff_summary " Stefan Beller
2016-09-13  4:46 ` Stefan Beller [this message]
2016-09-13 23:06   ` [RFC/PATCH 16/17] diff: buffer output in emit_line_0 Junio C Hamano
2016-09-13 23:28     ` Stefan Beller
2016-09-13 23:32       ` Junio C Hamano
2016-09-13 23:42         ` Stefan Beller
2016-09-13  4:46 ` [RFC/PATCH 17/17] diff.c: color moved lines differently Stefan Beller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160913044613.1037-17-sbeller@google.com \
    --to=sbeller@google.com \
    --cc=chriscool@tuxfamily.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).