git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 11/19] submodule.c: convert show_submodule_summary to use emit_line_fmt

From
Stefan Beller <sbeller@google.com>
Date
May 14, 2017, 04:01 UTC
Message-ID
<20170514040117.25865-12-sbeller@google.com>
In-Reply-To
<20170514040117.25865-1-sbeller@google.com>

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 function.

Signed-off-by: Stefan Beller <sbeller@google.com>
---
 diff.c      | 18 +++++++-------
 diff.h      |  5 ++++
 submodule.c | 78 ++++++++++++++++++++++++++++++-------------------------------
 submodule.h |  9 +++----
 4 files changed, 55 insertions(+), 55 deletions(-)
diff --git a/diff.c b/diff.c
index 369c804f03..45ec311828 100644
--- a/diff.c
+++ b/diff.c
@@ -546,15 +546,15 @@ static void emit_line_0(struct diff_options *o, const char *set, const char *res
 		fputc('\n', file);
 }
 
-static void emit_line(struct diff_options *o, const char *set, const char *reset,
-		      const char *line, int len)
+void emit_line(struct diff_options *o, const char *set, const char *reset,
+	       const char *line, int len)
 {
 	emit_line_0(o, set, reset, 0, line, len);
 }
 
-static void emit_line_fmt(struct diff_options *o,
-			  const char *set, const char *reset,
-			  const char *fmt, ...)
+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;
@@ -2379,8 +2379,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, &two->oid,
 				two->dirty_submodule,
 				meta, del, add, reset);
@@ -2390,11 +2389,10 @@ 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_inline_diff(o->file, one->path ? one->path : two->path,
-				line_prefix,
+		show_submodule_inline_diff(o, one->path ? one->path : two->path,
 				&one->oid, &two->oid,
 				two->dirty_submodule,
-				meta, del, add, reset, o);
+				meta, del, add, reset);
 		return;
 	}
 
diff --git a/diff.h b/diff.h
index 5be1ee77a7..addebd5a0f 100644
--- a/diff.h
+++ b/diff.h
@@ -188,6 +188,11 @@ struct diff_options {
 	int diff_path_counter;
 };
 
+void emit_line_fmt(struct diff_options *o, const char *set, const char *reset,
+		   const char *fmt, ...);
+void emit_line(struct diff_options *o, const char *set, const char *reset,
+	       const char *line, int len);
+
 enum color_diff {
 	DIFF_RESET = 0,
 	DIFF_CONTEXT = 1,
diff --git a/submodule.c b/submodule.c
index d3299e29c0..cfad469a2f 100644
--- a/submodule.c
+++ b/submodule.c
@@ -362,8 +362,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";
@@ -375,18 +375,12 @@ 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(o, del, reset, sb.buf, sb.len);
+		else if (add)
+			emit_line(o, add, reset, sb.buf, sb.len);
 	}
 	strbuf_release(&sb);
 }
@@ -413,8 +407,7 @@ void prepare_submodule_repo_env(struct argv_array *out)
  * attempt to lookup both the left and right commits and put them into the
  * left and right pointers.
  */
-static void show_submodule_header(FILE *f, const char *path,
-		const char *line_prefix,
+static void show_submodule_header(struct diff_options *o, const char *path,
 		struct object_id *one, struct object_id *two,
 		unsigned dirty_submodule, const char *meta,
 		const char *reset,
@@ -426,11 +419,11 @@ static void show_submodule_header(FILE *f, const char *path,
 	int fast_forward = 0, fast_backward = 0;
 
 	if (dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)
-		fprintf(f, "%sSubmodule %s contains untracked content\n",
-			line_prefix, path);
+		emit_line_fmt(o, NULL, NULL,
+			      "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, NULL, NULL,
+			      "Submodule %s contains modified content\n", path);
 
 	if (is_null_oid(one))
 		message = "(new submodule)";
@@ -472,21 +465,20 @@ static void show_submodule_header(FILE *f, const char *path,
 	}
 
 output_header:
-	strbuf_addf(&sb, "%s%sSubmodule %s ", line_prefix, meta, path);
+	strbuf_addf(&sb, "Submodule %s ", path);
 	strbuf_add_unique_abbrev(&sb, one->hash, DEFAULT_ABBREV);
 	strbuf_addstr(&sb, (fast_backward || fast_forward) ? ".." : "...");
 	strbuf_add_unique_abbrev(&sb, two->hash, 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(o, meta, reset, sb.buf, sb.len);
 
 	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,
 		struct object_id *one, struct object_id *two,
 		unsigned dirty_submodule, const char *meta,
 		const char *del, const char *add, const char *reset)
@@ -495,7 +487,7 @@ void show_submodule_summary(FILE *f, const char *path,
 	struct commit *left = NULL, *right = NULL;
 	struct commit_list *merge_bases = NULL;
 
-	show_submodule_header(f, path, line_prefix, one, two, dirty_submodule,
+	show_submodule_header(o, path, one, two, dirty_submodule,
 			      meta, reset, &left, &right, &merge_bases);
 
 	/*
@@ -508,11 +500,12 @@ void show_submodule_summary(FILE *f, const char *path,
 
 	/* Treat revision walker failure the same as missing commits */
 	if (prepare_submodule_summary(&rev, path, left, right, merge_bases)) {
-		fprintf(f, "%s(revision walker failed)\n", line_prefix);
+		const char *error = "(revision walker failed)\n";
+		emit_line(o, NULL, NULL, error, strlen(error));
 		goto out;
 	}
 
-	print_submodule_summary(&rev, f, line_prefix, del, add, reset);
+	print_submodule_summary(&rev, o, del, add, reset);
 
 out:
 	if (merge_bases)
@@ -521,20 +514,18 @@ void show_submodule_summary(FILE *f, const char *path,
 	clear_commit_marks(right, ~0);
 }
 
-void show_submodule_inline_diff(FILE *f, const char *path,
-		const char *line_prefix,
+void show_submodule_inline_diff(struct diff_options *o, const char *path,
 		struct object_id *one, struct object_id *two,
 		unsigned dirty_submodule, const char *meta,
-		const char *del, const char *add, const char *reset,
-		const struct diff_options *o)
+		const char *del, const char *add, const char *reset)
 {
 	const struct object_id *old = &empty_tree_oid, *new = &empty_tree_oid;
 	struct commit *left = NULL, *right = NULL;
 	struct commit_list *merge_bases = NULL;
-	struct strbuf submodule_dir = STRBUF_INIT;
 	struct child_process cp = CHILD_PROCESS_INIT;
+	struct strbuf sb = STRBUF_INIT;
 
-	show_submodule_header(f, path, line_prefix, one, two, dirty_submodule,
+	show_submodule_header(o, path, one, two, dirty_submodule,
 			      meta, reset, &left, &right, &merge_bases);
 
 	/* We need a valid left and right commit to display a difference */
@@ -547,15 +538,14 @@ void show_submodule_inline_diff(FILE *f, const char *path,
 	if (right)
 		new = two;
 
-	fflush(f);
 	cp.git_cmd = 1;
 	cp.dir = path;
-	cp.out = dup(fileno(f));
+	cp.out = -1;
 	cp.no_stdin = 1;
 
 	/* TODO: other options may need to be passed here. */
 	argv_array_push(&cp.args, "diff");
-	argv_array_pushf(&cp.args, "--line-prefix=%s", line_prefix);
+	argv_array_pushf(&cp.args, "--line-prefix=%s", diff_line_prefix(o));
 	if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
 		argv_array_pushf(&cp.args, "--src-prefix=%s%s/",
 				 o->b_prefix, path);
@@ -578,11 +568,21 @@ void show_submodule_inline_diff(FILE *f, const char *path,
 		argv_array_push(&cp.args, oid_to_hex(new));
 
 	prepare_submodule_repo_env(&cp.env_array);
-	if (run_command(&cp))
-		fprintf(f, "(diff failed)\n");
+	if (start_command(&cp)) {
+		const char *error = "(diff failed)\n";
+		emit_line(o, NULL, NULL, error, strlen(error));
+	}
+
+	while (strbuf_getwholeline_fd(&sb, cp.out, '\n') != EOF)
+		emit_line(o, NULL, NULL, sb.buf, sb.len);
+
+	if (finish_command(&cp)) {
+		const char *error = "(diff failed)\n";
+		emit_line(o, NULL, NULL, error, strlen(error));
+	}
 
 done:
-	strbuf_release(&submodule_dir);
+	strbuf_release(&sb);
 	if (merge_bases)
 		free_commit_list(merge_bases);
 	if (left)
diff --git a/submodule.h b/submodule.h
index 1277480add..9df0a3aea2 100644
--- a/submodule.h
+++ b/submodule.h
@@ -53,17 +53,14 @@ extern int parse_submodule_update_strategy(const char *value,
 		struct submodule_update_strategy *dst);
 extern const char *submodule_strategy_to_string(const struct submodule_update_strategy *s);
 extern void handle_ignore_submodules_arg(struct diff_options *, const char *);
-extern void show_submodule_summary(FILE *f, const char *path,
-		const char *line_prefix,
+extern void show_submodule_summary(struct diff_options *o, const char *path,
 		struct object_id *one, struct object_id *two,
 		unsigned dirty_submodule, const char *meta,
 		const char *del, const char *add, const char *reset);
-extern void show_submodule_inline_diff(FILE *f, const char *path,
-		const char *line_prefix,
+extern void show_submodule_inline_diff(struct diff_options *o, const char *path,
 		struct object_id *one, struct object_id *two,
 		unsigned dirty_submodule, const char *meta,
-		const char *del, const char *add, const char *reset,
-		const struct diff_options *opt);
+		const char *del, const char *add, const char *reset);
 extern void set_config_fetch_recurse_submodules(int value);
 extern void set_config_update_recurse_submodules(int value);
 /* Check if we want to update any submodule.*/

-- 
2.13.0.18.g183880de0a
Previous: Stefan BellerNext: Stefan Beller
Message 24 of 128 in “Diff machine: highlight moved lines.”
  1. 00/19 Diff machine: highlight moved lines.Stefan Beller, May 14, 2017
  2. 01/19 diff: readability fixStefan Beller, May 14, 2017
  3. 02/19 diff: move line ending check into emit_hunk_headerStefan Beller, May 14, 2017
  4. Junio C HamanoMay 15, 2017
  5. Stefan BellerMay 15, 2017
  6. 06/19 diff: add emit_line_fmtStefan Beller, May 14, 2017
  7. Brandon WilliamsMay 15, 2017
  8. 03/19 diff.c: drop 'nofirst' from emit_line_0Stefan Beller, May 14, 2017
  9. Jonathan TanMay 15, 2017
  10. Stefan BellerMay 15, 2017
  11. Jonathan TanMay 16, 2017
  12. Brandon WilliamsMay 15, 2017
  13. Stefan BellerMay 15, 2017
  14. Brandon WilliamsMay 15, 2017
  15. 08/19 diff.c: convert builtin_diff to use emit_line_*Stefan Beller, May 14, 2017
  16. Jonathan TanMay 15, 2017
  17. 10/19 diff.c: convert emit_rewrite_lines to use emit_line_*Stefan Beller, May 14, 2017
  18. Jonathan TanMay 15, 2017
  19. Stefan BellerMay 15, 2017
  20. 07/19 diff.c: convert fn_out_consume to use emit_line_*Stefan Beller, May 14, 2017
  21. Junio C HamanoMay 16, 2017
  22. Junio C HamanoMay 16, 2017
  23. Stefan BellerMay 16, 2017
  24. 11/19 submodule.c: convert show_submodule_summary to use emit_line_fmtStefan Beller, May 14, 2017
  25. 17/19 diff.c: factor out emit_line_ws for coloring whitespacesStefan Beller, May 14, 2017
  26. 14/19 diff.c: convert word diffing to use emit_line_*Stefan Beller, May 14, 2017
  27. Jonathan TanMay 15, 2017
  28. Stefan BellerMay 15, 2017
  29. 18/19 diff: buffer all output if asked toStefan Beller, May 14, 2017
  30. Jeff KingMay 14, 2017
  31. Stefan BellerMay 14, 2017
  32. Jonathan TanMay 16, 2017
  33. Stefan BellerMay 16, 2017
  34. 19/19 diff.c: color moved lines differentlyStefan Beller, May 14, 2017
  35. Brandon WilliamsMay 15, 2017
  36. Jonathan TanMay 16, 2017
  37. Jeff KingMay 16, 2017
  38. 12/19 diff.c: convert emit_binary_diff_body to use emit_line_*Stefan Beller, May 14, 2017
  39. 16/19 diff.c: convert diff_summary to use emit_line_*Stefan Beller, May 14, 2017
  40. 13/19 diff.c: convert show_stats to use emit_line_*Stefan Beller, May 14, 2017
  41. 09/19 diff.c: convert emit_rewrite_diff to use emit_line_*Stefan Beller, May 14, 2017
  42. 15/19 diff.c: convert diff_flush to use emit_line_*Stefan Beller, May 14, 2017
  43. Jonathan TanMay 15, 2017
  44. Stefan BellerMay 15, 2017
  45. 04/19 diff.c: factor out diff_flush_patch_all_file_pairsStefan Beller, May 14, 2017
  46. 05/19 diff.c: emit_line_0 can handle no color settingStefan Beller, May 14, 2017
  47. Jonathan TanMay 15, 2017
  48. Stefan BellerMay 15, 2017
  49. Junio C HamanoMay 15, 2017
  50. Stefan BellerMay 15, 2017
  51. 00/20 Diff machine: highlight moved lines.Stefan Beller, May 17, 2017
  52. 01/20 diff: readability fixStefan Beller, May 17, 2017
  53. 02/20 diff: move line ending check into emit_hunk_headerStefan Beller, May 17, 2017
  54. 03/20 diff.c: factor out diff_flush_patch_all_file_pairsStefan Beller, May 17, 2017
  55. 04/20 diff.c: teach emit_line_0 to accept sign parameterStefan Beller, May 17, 2017
  56. 07/20 diff.c: inline emit_line_0 into emit_lineStefan Beller, May 17, 2017
  57. 11/20 diff.c: convert emit_rewrite_lines to use emit_line_*Stefan Beller, May 17, 2017
  58. Junio C HamanoMay 17, 2017
  59. Stefan BellerMay 17, 2017
  60. Junio C HamanoMay 18, 2017
  61. 14/20 diff.c: convert show_stats to use emit_line_*Stefan Beller, May 17, 2017
  62. 06/20 diff.c: emit_line_0 takes parameter whether to output line prefixStefan Beller, May 17, 2017
  63. 08/20 diff.c: convert fn_out_consume to use emit_lineStefan Beller, May 17, 2017
  64. 17/20 diff.c: convert diff_summary to use emit_line_*Stefan Beller, May 17, 2017
  65. 16/20 diff.c: convert diff_flush to use emit_line_*Stefan Beller, May 17, 2017
  66. 20/20 diff.c: color moved lines differentlyStefan Beller, May 17, 2017
  67. 12/20 submodule.c: convert show_submodule_summary to use emit_line_fmtStefan Beller, May 17, 2017
  68. Junio C HamanoMay 17, 2017
  69. Stefan BellerMay 17, 2017
  70. Junio C HamanoMay 18, 2017
  71. Stefan BellerMay 18, 2017
  72. Junio C HamanoMay 20, 2017
  73. Stefan BellerMay 20, 2017
  74. 18/20 diff.c: emit_line includes whitespace highlightingStefan Beller, May 17, 2017
  75. 05/20 diff.c: emit_line_0 can handle no color settingStefan Beller, May 17, 2017
  76. 19/20 diff: buffer all output if asked toStefan Beller, May 17, 2017
  77. 13/20 diff.c: convert emit_binary_diff_body to use emit_line_*Stefan Beller, May 17, 2017
  78. 15/20 diff.c: convert word diffing to use emit_line_*Stefan Beller, May 17, 2017
  79. 09/20 diff.c: convert builtin_diff to use emit_line_*Stefan Beller, May 17, 2017
  80. 10/20 diff.c: convert emit_rewrite_diff to use emit_line_*Stefan Beller, May 17, 2017
  81. 00/20 Diff machine: highlight moved lines.Stefan Beller, May 18, 2017
  82. 01/20 diff: readability fixStefan Beller, May 18, 2017
  83. 03/20 diff.c: factor out diff_flush_patch_all_file_pairsStefan Beller, May 18, 2017
  84. 02/20 diff: move line ending check into emit_hunk_headerStefan Beller, May 18, 2017
  85. 11/20 diff.c: convert emit_rewrite_lines to use emit_line_*Stefan Beller, May 18, 2017
  86. 09/20 diff.c: convert builtin_diff to use emit_line_*Stefan Beller, May 18, 2017
  87. 10/20 diff.c: convert emit_rewrite_diff to use emit_line_*Stefan Beller, May 18, 2017
  88. 13/20 diff.c: convert emit_binary_diff_body to use emit_line_*Stefan Beller, May 18, 2017
  89. 12/20 submodule.c: convert show_submodule_summary to use emit_line_fmtStefan Beller, May 18, 2017
  90. 17/20 diff.c: convert diff_summary to use emit_line_*Stefan Beller, May 18, 2017
  91. 20/20 diff.c: color moved lines differentlyStefan Beller, May 18, 2017
  92. Jonathan TanMay 19, 2017
  93. Stefan BellerMay 19, 2017
  94. Jonathan TanMay 19, 2017
  95. 19/20 diff: buffer all output if asked toStefan Beller, May 18, 2017
  96. 18/20 diff.c: emit_line includes whitespace highlightingStefan Beller, May 18, 2017
  97. 15/20 diff.c: convert word diffing to use emit_line_*Stefan Beller, May 18, 2017
  98. 16/20 diff.c: convert diff_flush to use emit_line_*Stefan Beller, May 18, 2017
  99. 08/20 diff.c: convert fn_out_consume to use emit_lineStefan Beller, May 18, 2017
  100. 14/20 diff.c: convert show_stats to use emit_line_*Stefan Beller, May 18, 2017
  101. 07/20 diff.c: inline emit_line_0 into emit_lineStefan Beller, May 18, 2017
  102. 06/20 diff.c: emit_line_0 takes parameter whether to output line prefixStefan Beller, May 18, 2017
  103. 05/20 diff.c: emit_line_0 can handle no color settingStefan Beller, May 18, 2017
  104. 04/20 diff.c: teach emit_line_0 to accept sign parameterStefan Beller, May 18, 2017
  105. Jonathan TanMay 18, 2017
  106. Stefan BellerMay 22, 2017
  107. 00/17 Diff machine: highlight moved lines.Stefan Beller, May 23, 2017
  108. 01/17 diff: readability fixStefan Beller, May 23, 2017
  109. 02/17 diff: move line ending check into emit_hunk_headerStefan Beller, May 23, 2017
  110. 05/17 diff.c: convert fn_out_consume to use emit_lineStefan Beller, May 23, 2017
  111. 06/17 diff.c: convert builtin_diff to use emit_line_*Stefan Beller, May 23, 2017
  112. 07/17 diff.c: convert emit_rewrite_diff to use emit_line_*Stefan Beller, May 23, 2017
  113. 08/17 diff.c: convert emit_rewrite_lines to use emit_line_*Stefan Beller, May 23, 2017
  114. 09/17 submodule.c: convert show_submodule_summary to use emit_line_fmtStefan Beller, May 23, 2017
  115. Junio C HamanoMay 23, 2017
  116. Stefan BellerMay 23, 2017
  117. 11/17 diff.c: convert show_stats to use emit_line_*Stefan Beller, May 23, 2017
  118. 10/17 diff.c: convert emit_binary_diff_body to use emit_line_*Stefan Beller, May 23, 2017
  119. 16/17 diff: buffer all output if asked toStefan Beller, May 23, 2017
  120. 14/17 diff.c: convert diff_summary to use emit_line_*Stefan Beller, May 23, 2017
  121. 15/17 diff.c: emit_line includes whitespace highlightingStefan Beller, May 23, 2017
  122. 13/17 diff.c: convert diff_flush to use emit_line_*Stefan Beller, May 23, 2017
  123. 12/17 diff.c: convert word diffing to use emit_line_*Stefan Beller, May 23, 2017
  124. 04/17 diff: introduce more flexible emit functionStefan Beller, May 23, 2017
  125. 17/17 diff.c: color moved lines differentlyStefan Beller, May 23, 2017
  126. 03/17 diff.c: factor out diff_flush_patch_all_file_pairsStefan Beller, May 23, 2017
  127. Jacob KellerMay 27, 2017
  128. Stefan BellerMay 30, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.