On Sat, May 13, 2017 at 09:01:17PM -0700, Stefan Beller wrote:
> 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:
> 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:
> @@ -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
