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

[PATCHv5 00/17] Diff machine: highlight moved lines.

From
Stefan Beller <sbeller@google.com>
Date
May 24, 2017, 21:40 UTC
Message-ID
<20170524214036.29623-1-sbeller@google.com>
In-Reply-To
<20170523024048.16879-1-sbeller@google.com/>
v5:
* removed the color passing to the submodule to make the tests pass again.
* fixed an indentation issue that was introduced from v3 -> v4.
* I merged it with origin/next and tests pass here.

Thanks, Stefan

diff to v4:
diff --git a/diff.c b/diff.c
index 23e70d348e..1292d3c4ad 100644
--- a/diff.c
+++ b/diff.c
@@ -751,7 +751,7 @@ static void mark_color_as_moved(struct diff_options *o,
 }
 
 static void emit_diff_line(struct diff_options *o,
-				     struct diff_line *e)
+			   struct diff_line *e)
 {
 	const char *ws;
 	int has_trailing_newline, has_trailing_carriage_return;
@@ -804,7 +804,7 @@ static void emit_diff_line(struct diff_options *o,
 }
 
 static void append_diff_line(struct diff_options *o,
-				       struct diff_line *e)
+			     struct diff_line *e)
 {
 	struct diff_line *f;
 	ALLOC_GROW(o->line_buffer,
diff --git a/submodule.c b/submodule.c
index 428c996c97..19c63197fb 100644
--- a/submodule.c
+++ b/submodule.c
@@ -550,8 +550,6 @@ void show_submodule_inline_diff(struct diff_options *o, const char *path,
 
 	/* TODO: other options may need to be passed here. */
 	argv_array_push(&cp.args, "diff");
-	if (o->use_color)
-		argv_array_push(&cp.args, "--color=always");
 	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/",

v4:
* interdiff to v3 (what is currently origin/sb/diff-color-move) below.
* renamed the "buffered_patch_line" to "diff_line". Originally I planned
  to not carry the "line" part as it can be a piece of a line as well.
  But for the intended functionality it is best to keep the name.
  If we'd want to add more functionality to say have a move detection
  for words as well, we'd rename the struct to have a better name then.
  For now diff_line is the best. (Thanks Jonathan Nieder!)
* tests to demonstrate it doesn't mess with --color-words as well as
  submodules. (Thanks Jonathan Tan!)
* added in the statics (Thanks Ramsay!)
* smaller scope for the hashmaps (Thanks Jonathan Tan!)
* some commit messages were updated, prior patch 4-7 is squashed into one
  (Thanks Jonathan Tan!)
* the tests added revealed an actual fault: now that the submodule process
  is not attached to a dupe of our stdout, it would stop coloring the
  output. We need to pass on use-color explicitly.
* updated the NEEDSWORK comment in the second last patch.

Thanks for bearing,
Stefan

v3:
* see interdiff below.
* fixing one invalid computation (Thanks Junio!)
* I reasoned more about submodule and word diffing, see the commit message
  of the last patch:
  
    A note on the options '--submodule=diff' and '--color-words/--word-diff':
    In the conversion to use emit_line in the prior patches both submodules
    as well as word diff output carefully chose to call emit_line with sign=0.
    All output with sign=0 is ignored for move detection purposes in this
    patch, such that no weird looking output will be generated for these
    cases. This leads to another thought: We could pass on '--color-moved' to
    submodules such that they color up moved lines for themselves. If we'd do
    so only line moves within a repository boundary are marked up.

* better name for emit_line outside of diff.[ch]

v2:
* emit_line now takes an argument that indicates if we want it
  to emit the line prefix as well. This should allow for a more faithful
  refactoring in the beginning. (Thanks Jonathan!)
* fixed memleaks (Thanks Brandon!)
* "git -c color.moved=true log -p" works now! (Thanks Jeff)
* interdiff below, though it is large.
* less intrusive than v1 (Thanks Jonathan!)

v1:

For details on *why* see the commit message of the last commit.

The first five patches are slight refactorings to get into good
shape, the next patches are funneling all output through emit_line_*.

The second last patch introduces an option to buffer up all output
before printing, and then the last patch can color up moved lines
of code.

Any feedback welcome.

Thanks,
Stefan

Stefan Beller (17):
  diff: readability fix
  diff: move line ending check into emit_hunk_header
  diff.c: factor out diff_flush_patch_all_file_pairs
  diff: introduce more flexible emit function
  diff.c: convert fn_out_consume to use emit_line
  diff.c: convert builtin_diff to use emit_line_*
  diff.c: convert emit_rewrite_diff to use emit_line_*
  diff.c: convert emit_rewrite_lines to use emit_line_*
  submodule.c: convert show_submodule_summary to use emit_line_fmt
  diff.c: convert emit_binary_diff_body to use emit_line_*
  diff.c: convert show_stats to use emit_line_*
  diff.c: convert word diffing to use emit_line_*
  diff.c: convert diff_flush to use emit_line_*
  diff.c: convert diff_summary to use emit_line_*
  diff.c: emit_line includes whitespace highlighting
  diff: buffer all output if asked to
  diff.c: color moved lines differently

 Documentation/config.txt   |  14 +-
 diff.c                     | 858 +++++++++++++++++++++++++++++++++------------
 diff.h                     |  59 +++-
 submodule.c                |  87 ++---
 submodule.h                |   9 +-
 t/t4015-diff-whitespace.sh | 267 ++++++++++++++
 6 files changed, 1016 insertions(+), 278 deletions(-)

-- 
2.13.0.18.g7d86cc8ba0
Next: Stefan Beller
Message 1 of 125 in “[PATCHv5 00/17] Diff machine: highlight moved lines.”
  1. Stefan BellerMay 24, 2017
  2. 01/17 diff: readability fixStefan Beller, May 24, 2017
  3. 03/17 diff.c: factor out diff_flush_patch_all_file_pairsStefan Beller, May 24, 2017
  4. 09/17 submodule.c: convert show_submodule_summary to use emit_line_fmtStefan Beller, May 24, 2017
  5. 04/17 diff: introduce more flexible emit functionStefan Beller, May 24, 2017
  6. Jonathan TanJun 13, 2017
  7. Stefan BellerJun 13, 2017
  8. Jonathan TanJun 13, 2017
  9. 05/17 diff.c: convert fn_out_consume to use emit_lineStefan Beller, May 24, 2017
  10. 02/17 diff: move line ending check into emit_hunk_headerStefan Beller, May 24, 2017
  11. 07/17 diff.c: convert emit_rewrite_diff to use emit_line_*Stefan Beller, May 24, 2017
  12. 15/17 diff.c: emit_line includes whitespace highlightingStefan Beller, May 24, 2017
  13. 11/17 diff.c: convert show_stats to use emit_line_*Stefan Beller, May 24, 2017
  14. 17/17 diff.c: color moved lines differentlyStefan Beller, May 24, 2017
  15. Junio C HamanoMay 25, 2017
  16. Stefan BellerMay 25, 2017
  17. 10/17 diff.c: convert emit_binary_diff_body to use emit_line_*Stefan Beller, May 24, 2017
  18. 08/17 diff.c: convert emit_rewrite_lines to use emit_line_*Stefan Beller, May 24, 2017
  19. 16/17 diff: buffer all output if asked toStefan Beller, May 24, 2017
  20. Junio C HamanoMay 25, 2017
  21. Stefan BellerMay 25, 2017
  22. Junio C HamanoMay 26, 2017
  23. Jonathan TanJun 13, 2017
  24. Stefan BellerJun 14, 2017
  25. 12/17 diff.c: convert word diffing to use emit_line_*Stefan Beller, May 24, 2017
  26. 14/17 diff.c: convert diff_summary to use emit_line_*Stefan Beller, May 24, 2017
  27. 13/17 diff.c: convert diff_flush to use emit_line_*Stefan Beller, May 24, 2017
  28. 06/17 diff.c: convert builtin_diff to use emit_line_*Stefan Beller, May 24, 2017
  29. Junio C HamanoMay 25, 2017
  30. Stefan BellerMay 25, 2017
  31. Junio C HamanoMay 26, 2017
  32. Stefan BellerMay 26, 2017
  33. 0/1 Diff machine: highlight moved lines.Stefan Beller, May 27, 2017
  34. 1/1 diff.c: color moved lines differentlyStefan Beller, May 27, 2017
  35. Philip OakleyMay 27, 2017
  36. Stefan BellerMay 30, 2017
  37. diff.c: color moved lines differentlyStefan Beller, Jun 1, 2017
  38. Jonathan TanJun 13, 2017
  39. Stefan BellerJun 14, 2017
  40. 00/26 reroll of sb/diff-color-movedStefan Beller, Jun 20, 2017
  41. 02/26 diff.c: move line ending check into emit_hunk_headerStefan Beller, Jun 20, 2017
  42. 03/26 diff.c: factor out diff_flush_patch_all_file_pairsStefan Beller, Jun 20, 2017
  43. 06/26 diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_FRAGINFOStefan Beller, Jun 20, 2017
  44. 07/26 diff.c: emit_diff_symbol learns DIFF_SYMBOL_NO_LF_EOFStefan Beller, Jun 20, 2017
  45. 08/26 diff.c: migrate emit_line_checked to use emit_diff_symbolStefan Beller, Jun 20, 2017
  46. Junio C HamanoJun 21, 2017
  47. Stefan BellerJun 22, 2017
  48. Stefan BellerJun 22, 2017
  49. Junio C HamanoJun 23, 2017
  50. 11/26 diff.c: emit_diff_symbol learns DIFF_SYMBOL_FILEPAIRStefan Beller, Jun 20, 2017
  51. Jonathan TanJun 20, 2017
  52. Junio C HamanoJun 21, 2017
  53. Stefan BellerJun 22, 2017
  54. 04/26 diff.c: introduce emit_diff_symbolStefan Beller, Jun 20, 2017
  55. Junio C HamanoJun 21, 2017
  56. Stefan BellerJun 21, 2017
  57. Junio C HamanoJun 21, 2017
  58. Junio C HamanoJun 21, 2017
  59. Stefan BellerJun 21, 2017
  60. Junio C HamanoJun 21, 2017
  61. Stefan BellerJun 21, 2017
  62. Brandon WilliamsJun 21, 2017
  63. Junio C HamanoJun 21, 2017
  64. Brandon WilliamsJun 21, 2017
  65. 15/26 submodule.c: migrate diff output to use emit_diff_symbolStefan Beller, Jun 20, 2017
  66. Jonathan TanJun 20, 2017
  67. 19/26 diff.c: emit_diff_symbol learns about DIFF_SYMBOL_STAT_SEPStefan Beller, Jun 20, 2017
  68. 12/26 diff.c: emit_diff_symbol learns DIFF_SYMBOL_HEADERStefan Beller, Jun 20, 2017
  69. 17/26 diff.c: convert show_stats to use emit_diff_symbolStefan Beller, Jun 20, 2017
  70. Brandon WilliamsJun 21, 2017
  71. Stefan BellerJun 21, 2017
  72. 18/26 diff.c: convert word diffing to use emit_diff_symbolStefan Beller, Jun 20, 2017
  73. 25/26 diff: document the new --color-moved settingStefan Beller, Jun 20, 2017
  74. 14/26 diff.c: emit_diff_symbol learns DIFF_SYMBOL_REWRITE_DIFFStefan Beller, Jun 20, 2017
  75. 26/26 diff.c: have a "machine parseable" move coloringStefan Beller, Jun 20, 2017
  76. Stefan BellerJun 20, 2017
  77. Ævar Arnfjörð BjarmasonJun 23, 2017
  78. Brandon WilliamsJun 21, 2017
  79. Junio C HamanoJun 21, 2017
  80. Stefan BellerJun 21, 2017
  81. 20/26 diff.c: emit_diff_symbol learns about DIFF_SYMBOL_SUMMARYStefan Beller, Jun 20, 2017
  82. 21/26 diff.c: buffer all output if asked toStefan Beller, Jun 20, 2017
  83. 24/26 diff.c: add dimming to moved line detectionStefan Beller, Jun 20, 2017
  84. Junio C HamanoJun 21, 2017
  85. 16/26 diff.c: convert emit_binary_diff_body to use emit_diff_symbolStefan Beller, Jun 20, 2017
  86. Junio C HamanoJun 21, 2017
  87. 23/26 diff.c: color moved lines differently, plain modeStefan Beller, Jun 20, 2017
  88. 09/26 diff.c: emit_diff_symbol learns DIFF_SYMBOL_WORDS{_PORCELAIN}Stefan Beller, Jun 20, 2017
  89. 22/26 diff.c: color moved lines differentlyStefan Beller, Jun 20, 2017
  90. Jonathan TanJun 20, 2017
  91. Stefan BellerJun 20, 2017
  92. 05/26 diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_MARKERStefan Beller, Jun 20, 2017
  93. 10/26 diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_INCOMPLETEStefan Beller, Jun 20, 2017
  94. 13/26 diff.c: emit_diff_symbol learns about DIFF_SYMBOL_BINARY_FILESStefan Beller, Jun 20, 2017
  95. Junio C HamanoJun 21, 2017
  96. Stefan BellerJun 21, 2017
  97. 01/26 diff.c: readability fixStefan Beller, Jun 20, 2017
  98. 00/25 reroll of sb/diff-color-movedStefan Beller, Jun 23, 2017
  99. 02/25 diff.c: move line ending check into emit_hunk_headerStefan Beller, Jun 23, 2017
  100. 04/25 diff.c: introduce emit_diff_symbolStefan Beller, Jun 23, 2017
  101. Junio C HamanoJun 23, 2017
  102. Stefan BellerJun 23, 2017
  103. 06/25 diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_FRAGINFOStefan Beller, Jun 23, 2017
  104. 03/25 diff.c: factor out diff_flush_patch_all_file_pairsStefan Beller, Jun 23, 2017
  105. 05/25 diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_MARKERStefan Beller, Jun 23, 2017
  106. 09/25 diff.c: emit_diff_symbol learns DIFF_SYMBOL_WORDS[_PORCELAIN]Stefan Beller, Jun 23, 2017
  107. 07/25 diff.c: emit_diff_symbol learns DIFF_SYMBOL_NO_LF_EOFStefan Beller, Jun 23, 2017
  108. 13/25 diff.c: emit_diff_symbol learns about DIFF_SYMBOL_BINARY_FILESStefan Beller, Jun 23, 2017
  109. 12/25 diff.c: emit_diff_symbol learns DIFF_SYMBOL_HEADERStefan Beller, Jun 23, 2017
  110. 14/25 diff.c: emit_diff_symbol learns DIFF_SYMBOL_REWRITE_DIFFStefan Beller, Jun 23, 2017
  111. 23/25 diff.c: color moved lines differently, plain modeStefan Beller, Jun 23, 2017
  112. 22/25 diff.c: color moved lines differentlyStefan Beller, Jun 23, 2017
  113. 15/25 submodule.c: migrate diff output to use emit_diff_symbolStefan Beller, Jun 23, 2017
  114. 25/25 diff: document the new --color-moved settingStefan Beller, Jun 23, 2017
  115. 11/25 diff.c: emit_diff_symbol learns DIFF_SYMBOL_FILEPAIR_{PLUS, MINUS}Stefan Beller, Jun 23, 2017
  116. 24/25 diff.c: add dimming to moved line detectionStefan Beller, Jun 23, 2017
  117. 01/25 diff.c: readability fixStefan Beller, Jun 23, 2017
  118. 10/25 diff.c: emit_diff_symbol learns DIFF_SYMBOL_CONTEXT_INCOMPLETEStefan Beller, Jun 23, 2017
  119. 21/25 diff.c: buffer all output if asked toStefan Beller, Jun 23, 2017
  120. 08/25 diff.c: migrate emit_line_checked to use emit_diff_symbolStefan Beller, Jun 23, 2017
  121. 18/25 diff.c: convert word diffing to use emit_diff_symbolStefan Beller, Jun 23, 2017
  122. 17/25 diff.c: convert show_stats to use emit_diff_symbolStefan Beller, Jun 23, 2017
  123. 19/25 diff.c: emit_diff_symbol learns about DIFF_SYMBOL_STAT_SEPStefan Beller, Jun 23, 2017
  124. 20/25 diff.c: emit_diff_symbol learns about DIFF_SYMBOL_SUMMARYStefan Beller, Jun 23, 2017
  125. 16/25 diff.c: convert emit_binary_diff_body to use emit_diff_symbolStefan Beller, Jun 23, 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.