Hi Stefan,

On Fri, 10 Aug 2018, Stefan Beller wrote:

> The range-diff coloring is a bit fuzzy when it comes to special lines of
> a diff, such as indicating new and old files with +++ and ---, as it
> would pickup the first character and interpret it for its coloring, which
> seems annoying as in regular diffs, these lines are colored bold via
> DIFF_METAINFO.
> 
> By indenting these lines by a white space, they will be treated as context
> which is much more useful, an example [1] on the range diff series itself:
> 
> [...]
>     + diff --git a/Documentation/git-range-diff.txt 
> b/Documentation/git-range-diff.txt
>     + new file mode 100644
>     + --- /dev/null
>     + +++ b/Documentation/git-range-diff.txt
>     +@@
>     ++git-range-diff(1)
> [...]
>     +
>       diff --git a/Makefile b/Makefile
>       --- a/Makefile
>       +++ b/Makefile
> [...]
> 
> The first lines that introduce the new file for the man page will have the
> '+' sign colored and the rest of the line will be bold.
> 
> The later lines that indicate a change to the Makefile will be treated as
> context both in the outer and inner diff, such that those lines stay
> regular color.

While I am a fan of having those lines colored correctly, I have to admit
that I am not exactly enthusiastic about that extra indentation...

Otherwise, this looks good to me.

Thanks,
Dscho

> 
> [1] ./git-range-diff pr-1/dscho/branch-diff-v3...pr-1/dscho/branch-diff-v4
>     These tags are found at https://github.com/gitgitgadget/git
> 
> Signed-off-by: Stefan Beller <sbel...@google.com>
> ---
>  range-diff.c          |  2 ++
>  t/t3206-range-diff.sh | 12 ++++++------
>  2 files changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/range-diff.c b/range-diff.c
> index 6e4192c8e79..a8db7c4f8d3 100644
> --- a/range-diff.c
> +++ b/range-diff.c
> @@ -85,6 +85,7 @@ static int read_patches(const char *range, struct 
> string_list *list)
>                       strbuf_addch(&buf, '\n');
>                       if (!util->diff_offset)
>                               util->diff_offset = buf.len;
> +                     strbuf_addch(&buf, ' ');
>                       strbuf_addbuf(&buf, &line);
>               } else if (in_header) {
>                       if (starts_with(line.buf, "Author: ")) {
> @@ -121,6 +122,7 @@ static int read_patches(const char *range, struct 
> string_list *list)
>                       strbuf_addch(&buf, ' ');
>                       strbuf_add(&buf, line.buf + 1, line.len - 1);
>               } else {
> +                     strbuf_addch(&buf, ' ');
>                       strbuf_addbuf(&buf, &line);
>               }
>  
> diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
> index 7dc7c80a1de..c94f9bf5ee1 100755
> --- a/t/t3206-range-diff.sh
> +++ b/t/t3206-range-diff.sh
> @@ -133,9 +133,9 @@ test_expect_success 'changed message' '
>           Z
>           +    Also a silly comment here!
>           +
> -         Zdiff --git a/file b/file
> -         Z--- a/file
> -         Z+++ b/file
> +         Z diff --git a/file b/file
> +         Z --- a/file
> +         Z +++ b/file
>       3:  147e64e = 3:  b9cb956 s/11/B/
>       4:  a63e992 = 4:  8add5f1 s/12/B/
>       EOF
> @@ -152,9 +152,9 @@ test_expect_success 'dual-coloring' '
>       :     <RESET>
>       :    <REVERSE><GREEN>+<RESET><BOLD>    Also a silly comment here!<RESET>
>       :    <REVERSE><GREEN>+<RESET>
> -     :     diff --git a/file b/file<RESET>
> -     :    <RED> --- a/file<RESET>
> -     :    <GREEN> +++ b/file<RESET>
> +     :      diff --git a/file b/file<RESET>
> +     :      --- a/file<RESET>
> +     :      +++ b/file<RESET>
>       :<RED>3:  0559556 <RESET><YELLOW>!<RESET><GREEN> 3:  
> b9cb956<RESET><YELLOW> s/11/B/<RESET>
>       :    <REVERSE><CYAN>@@ -10,7 +10,7 @@<RESET>
>       :      9<RESET>
> -- 
> 2.18.0.865.gffc8e1a3cd6-goog
> 
> 

Reply via email to