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

Re: [PATCH 14/19] diff.c: convert word diffing to use emit_line_*

From
JTJonathan Tan <jonathantanmy@google.com>
Date
May 15, 2017, 22:40 UTC
Message-ID
<f49dc4f8-cc83-934f-e582-79efdc613f70@google.com>
In-Reply-To
<20170514040117.25865-15-sbeller@google.com>
On 05/13/2017 09:01 PM, Stefan Beller wrote:
Show 66 quoted lines
> 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 covers all code related to diffing words.
>
> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
>  diff.c | 56 ++++++++++++++++++++++++++++----------------------------
>  1 file changed, 28 insertions(+), 28 deletions(-)
>
> diff --git a/diff.c b/diff.c
> index 91dc045a45..07041a35ad 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -886,37 +886,38 @@ struct diff_words_data {
>  	struct diff_words_style *style;
>  };
>
> -static int fn_out_diff_words_write_helper(FILE *fp,
> +static int fn_out_diff_words_write_helper(struct diff_options *o,
>  					  struct diff_words_style_elem *st_el,
>  					  const char *newline,
>  					  size_t count, const char *buf,
>  					  const char *line_prefix)
>  {
> -	int print = 0;
> +	struct strbuf sb = STRBUF_INIT;
>
>  	while (count) {
>  		char *p = memchr(buf, '\n', count);
> -		if (print)
> -			fputs(line_prefix, fp);
> +
>  		if (p != buf) {
> -			if (st_el->color && fputs(st_el->color, fp) < 0)
> -				return -1;
> -			if (fputs(st_el->prefix, fp) < 0 ||
> -			    fwrite(buf, p ? p - buf : count, 1, fp) != 1 ||
> -			    fputs(st_el->suffix, fp) < 0)
> -				return -1;
> -			if (st_el->color && *st_el->color
> -			    && fputs(GIT_COLOR_RESET, fp) < 0)
> -				return -1;
> +			if (st_el->color)
> +				strbuf_addstr(&sb, st_el->color);
> +			strbuf_addstr(&sb, st_el->prefix);
> +			strbuf_add(&sb, buf, p ? p - buf : count);
> +			strbuf_addstr(&sb, st_el->suffix);
> +			if (st_el->color && *st_el->color)
> +			    strbuf_addstr(&sb, GIT_COLOR_RESET);
>  		}
>  		if (!p)
> -			return 0;
> -		if (fputs(newline, fp) < 0)
> -			return -1;
> +			goto out;
> +		strbuf_addstr(&sb, newline);
> +		emit_line(o, NULL, NULL, sb.buf, sb.len);

I suspect that this will need to be refactored more thoroughly. Here, for example, emit_line (which prints the prefix) is printed nearly unconditionally, whereas in the original version, "fputs(line_prefix, fp)" is only printed when "print" is true.

Show 67 quoted lines
> +		strbuf_reset(&sb);
>  		count -= p + 1 - buf;
>  		buf = p + 1;
> -		print = 1;
>  	}
> +out:
> +	if (sb.len)
> +		emit_line(o, NULL, NULL, sb.buf, sb.len);
> +	strbuf_release(&sb);
>  	return 0;
>  }
>
> @@ -994,25 +995,25 @@ static void fn_out_diff_words_aux(void *priv, char *line, unsigned long len)
>  	} else
>  		plus_begin = plus_end = diff_words->plus.orig[plus_first].end;
>
> -	if (color_words_output_graph_prefix(diff_words)) {
> -		fputs(line_prefix, diff_words->opt->file);
> -	}
> +	if (color_words_output_graph_prefix(diff_words))
> +		emit_line(diff_words->opt, NULL, NULL, "", 0);
> +
>  	if (diff_words->current_plus != plus_begin) {
> -		fn_out_diff_words_write_helper(diff_words->opt->file,
> +		fn_out_diff_words_write_helper(diff_words->opt,
>  				&style->ctx, style->newline,
>  				plus_begin - diff_words->current_plus,
>  				diff_words->current_plus, line_prefix);
>  		if (*(plus_begin - 1) == '\n')
> -			fputs(line_prefix, diff_words->opt->file);
> +			emit_line(diff_words->opt, NULL, NULL, "", 0);
>  	}
>  	if (minus_begin != minus_end) {
> -		fn_out_diff_words_write_helper(diff_words->opt->file,
> +		fn_out_diff_words_write_helper(diff_words->opt,
>  				&style->old, style->newline,
>  				minus_end - minus_begin, minus_begin,
>  				line_prefix);
>  	}
>  	if (plus_begin != plus_end) {
> -		fn_out_diff_words_write_helper(diff_words->opt->file,
> +		fn_out_diff_words_write_helper(diff_words->opt,
>  				&style->new, style->newline,
>  				plus_end - plus_begin, plus_begin,
>  				line_prefix);
> @@ -1109,8 +1110,7 @@ static void diff_words_show(struct diff_words_data *diff_words)
>
>  	/* special case: only removal */
>  	if (!diff_words->plus.text.size) {
> -		fputs(line_prefix, diff_words->opt->file);
> -		fn_out_diff_words_write_helper(diff_words->opt->file,
> +		fn_out_diff_words_write_helper(diff_words->opt,
>  			&style->old, style->newline,
>  			diff_words->minus.text.size,
>  			diff_words->minus.text.ptr, line_prefix);
> @@ -1136,8 +1136,8 @@ static void diff_words_show(struct diff_words_data *diff_words)
>  	if (diff_words->current_plus != diff_words->plus.text.ptr +
>  			diff_words->plus.text.size) {
>  		if (color_words_output_graph_prefix(diff_words))
> -			fputs(line_prefix, diff_words->opt->file);
> -		fn_out_diff_words_write_helper(diff_words->opt->file,
> +			emit_line(diff_words->opt, NULL, NULL, "", 0);
> +		fn_out_diff_words_write_helper(diff_words->opt,
>  			&style->ctx, style->newline,
>  			diff_words->plus.text.ptr + diff_words->plus.text.size
>  			- diff_words->current_plus, diff_words->current_plus,
>
Previous: Stefan BellerNext: Stefan Beller
Message 27 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.