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

Re: [PATCH 19/19] diff.c: color moved lines differently

From
Jeff King <peff@peff.net>
Date
May 16, 2017, 12:31 UTC
Message-ID
<20170516123132.gekwt6fjbmadubnp@sigill.intra.peff.net>
In-Reply-To
<20170514040117.25865-20-sbeller@google.com>
On Sat, May 13, 2017 at 09:01:17PM -0700, Stefan Beller wrote:
Show 14 quoted lines
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 475e874d51..90403c06e3 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1051,14 +1051,22 @@ This does not affect linkgit:git-format-patch[1] or the
>  'git-diff-{asterisk}' plumbing commands.  Can be overridden on the
>  command line with the `--color[=<when>]` option.
>  
> +color.moved::
> +	A boolean value, whether a diff should color moved lines
> +	differently. The moved lines are searched for in the diff only.
> +	Duplicated lines from somewhere in the project that are not
> +	part of the diff are not colored as moved.
> +	Defaults to false.

I wanted to play with this series to see how it looked on a few commits. Since this was the only documentation change, I tried "git -c color.moved=true diff ...". But it doesn't seem to work.

If we grep for diff_color_moved_default, we can see it declared:
Show 12 quoted lines
> diff --git a/diff.c b/diff.c
> index dbab7fb44e..6372e0eb25 100644
> --- a/diff.c
> +++ b/diff.c
> @@ -31,6 +31,7 @@ static int diff_indent_heuristic; /* experimental */
>  static int diff_rename_limit_default = 400;
>  static int diff_suppress_blank_empty;
>  static int diff_use_color_default = -1;
> +static int diff_color_moved_default;
>  static int diff_context_default = 3;
>  static int diff_interhunk_context_default;
>  static const char *diff_word_regex_cfg;
and we can see it parsed:
Show 8 quoted lines
> @@ -234,6 +247,10 @@ int git_diff_ui_config(const char *var, const char *value, void *cb)
>  		diff_use_color_default = git_config_colorbool(var, value);
>  		return 0;
>  	}
> +	if (!strcmp(var, "color.moved")) {
> +		diff_color_moved_default = git_config_bool(var, value);
> +		return 0;
> +	}
But then nobody uses it. ;)

I suspect diff_setup() needs to copy the default into the diff_options struct?

By reading the code, I found --color-moved, which did work. Yay. So then I wanted to see how it looked on a wide variety of commits. So I ran "git log -p --color-moved", but it segfaulted. :-/

I didn't dig, but here's what valgrind says, in case it helps:
  $ valgrind ./git --no-pager log --oneline -p --color-moved
  [...]
  fcdb8874d diff: buffer all output if asked to
  ==8801== Invalid write of size 4
  ==8801==    at 0x1FD521: builtin_diff (diff.c:2869)
  ==8801==    by 0x1FF3CB: run_diff_cmd (diff.c:3582)
  ==8801==    by 0x1FF7F9: run_diff (diff.c:3670)
  ==8801==    by 0x202CC8: diff_flush_patch (diff.c:4664)
  ==8801==    by 0x204123: diff_flush_patch_all_file_pairs (diff.c:5125)
  ==8801==    by 0x204682: diff_flush (diff.c:5249)
  ==8801==    by 0x22CE18: log_tree_diff_flush (log-tree.c:775)
  ==8801==    by 0x22D063: log_tree_diff (log-tree.c:842)
  ==8801==    by 0x22D195: log_tree_commit (log-tree.c:871)
  ==8801==    by 0x1648CE: cmd_log_walk (log.c:365)
  ==8801==    by 0x1659B0: cmd_log (log.c:689)
  ==8801==    by 0x11A434: run_builtin (git.c:371)
  ==8801==  Address 0x8 is not stack'd, malloc'd or (recently) free'd
  ==8801== 
  ==8801== 
  ==8801== Process terminating with default action of signal 11
  (SIGSEGV)
  ==8801==  Access not within mapped region at address 0x8
  ==8801==    at 0x1FD521: builtin_diff (diff.c:2869)
  ==8801==    by 0x1FF3CB: run_diff_cmd (diff.c:3582)
  ==8801==    by 0x1FF7F9: run_diff (diff.c:3670)
  ==8801==    by 0x202CC8: diff_flush_patch (diff.c:4664)
  ==8801==    by 0x204123: diff_flush_patch_all_file_pairs (diff.c:5125)
  ==8801==    by 0x204682: diff_flush (diff.c:5249)
  ==8801==    by 0x22CE18: log_tree_diff_flush (log-tree.c:775)
  ==8801==    by 0x22D063: log_tree_diff (log-tree.c:842)
  ==8801==    by 0x22D195: log_tree_commit (log-tree.c:871)
  ==8801==    by 0x1648CE: cmd_log_walk (log.c:365)
  ==8801==    by 0x1659B0: cmd_log (log.c:689)
  ==8801==    by 0x11A434: run_builtin (git.c:371)

It does show one commit correctly; fcdb8874d (your commit, but picked up from the list, so my commit sha1) is the second one. I don't know if that's coincidence, or maybe there's something that's not properly between diff runs.

-Peff
Previous: Jonathan TanNext: Stefan Beller
Message 37 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.