{"thread":{"id":"49096","subject":"[PATCH 0/4] Better colors in range-diff!","startedAt":"2018-08-10T22:49:36Z","lastAt":"2018-08-23T14:26:21Z","messageCount":27,"participants":["Stefan Beller","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"355203","messageId":"20180810224923.143625-1-sbeller@google.com","threadId":"49096","inReplyTo":null,"subject":"[PATCH 0/4] Better colors in range-diff!","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-10T22:49:19Z","receivedAt":"2018-08-10T22:49:36Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This improves colors of the range-diff, see last patch for details.\n\nThis is a partial resend of\nhttps://public-inbox.org/git/20180804015317.182683-1-sbeller@google.com/\nand is also available via\n\n  git fetch https://github.com/stefanbeller/git sb/range-diff-better-colors\n\nIt applies on the (just reset) series of sb/range-diff-colors.\n\nThanks,\nStefan\n\nStefan Beller (4):\n  diff.c: emit_line_0 to take string instead of first sign\n  diff.c: add --output-indicator-{new, old, context}\n  range-diff: make use of different output indicators\n  range-diff: indent special lines as context\n\n diff.c                | 43 +++++++++++++++++++++++++++++++------------\n diff.h                |  5 +++++\n range-diff.c          | 17 ++++++++++++++++-\n t/t3206-range-diff.sh | 12 ++++++------\n 4 files changed, 58 insertions(+), 19 deletions(-)\n\n-- \n2.18.0.865.gffc8e1a3cd6-goog\n\n"},{"id":"355204","messageId":"20180810224923.143625-2-sbeller@google.com","threadId":"49096","inReplyTo":"20180810224923.143625-1-sbeller@google.com","subject":"[PATCH 1/4] diff.c: emit_line_0 to take string instead of first sign","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-10T22:49:20Z","receivedAt":"2018-08-10T22:49:39Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"By providing a string as the first part of the emission we can extend\nit later more easily.\n\nWhile at it, document emit_line_0.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n diff.c | 28 +++++++++++++++++-----------\n 1 file changed, 17 insertions(+), 11 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex af9316c8f91..b3cb73eb69a 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -621,9 +621,15 @@ static void check_blank_at_eof(mmfile_t *mf1, mmfile_t *mf2,\n \tecbdata->blank_at_eof_in_postimage = (at - l2) + 1;\n }\n \n+/*\n+ * Emits\n+ * <set_sign> <first> <reset> <set> <second> <reset> LF\n+ * if they are present. 'first' is a NULL terminated string,\n+ * 'second' is a buffer of length 'len'.\n+ */\n static void emit_line_0(struct diff_options *o,\n \t\t\tconst char *set_sign, const char *set, const char *reset,\n-\t\t\tint first, const char *line, int len)\n+\t\t\tconst char *first, const char *second, int len)\n {\n \tint has_trailing_newline, has_trailing_carriage_return;\n \tint reverse = !!set && !!set_sign;\n@@ -633,11 +639,11 @@ static void emit_line_0(struct diff_options *o,\n \n \tfputs(diff_line_prefix(o), file);\n \n-\thas_trailing_newline = (len > 0 && line[len-1] == '\\n');\n+\thas_trailing_newline = (len > 0 && second[len-1] == '\\n');\n \tif (has_trailing_newline)\n \t\tlen--;\n \n-\thas_trailing_carriage_return = (len > 0 && line[len-1] == '\\r');\n+\thas_trailing_carriage_return = (len > 0 && second[len-1] == '\\r');\n \tif (has_trailing_carriage_return)\n \t\tlen--;\n \n@@ -655,7 +661,7 @@ static void emit_line_0(struct diff_options *o,\n \t}\n \n \tif (first)\n-\t\tfputc(first, file);\n+\t\tfputs(first, file);\n \n \tif (!len)\n \t\tgoto end_of_line;\n@@ -666,7 +672,7 @@ static void emit_line_0(struct diff_options *o,\n \t\tfputs(set, file);\n \t\tneeds_reset = 1;\n \t}\n-\tfwrite(line, len, 1, file);\n+\tfwrite(second, len, 1, file);\n \tneeds_reset |= len > 0;\n \n end_of_line:\n@@ -681,7 +687,7 @@ static void emit_line_0(struct diff_options *o,\n static void emit_line(struct diff_options *o, const char *set, const char *reset,\n \t\t      const char *line, int len)\n {\n-\temit_line_0(o, set, NULL, reset, 0, line, len);\n+\temit_line_0(o, set, NULL, reset, NULL, line, len);\n }\n \n enum diff_symbol {\n@@ -1201,7 +1207,7 @@ static void dim_moved_lines(struct diff_options *o)\n static void emit_line_ws_markup(struct diff_options *o,\n \t\t\t\tconst char *set_sign, const char *set,\n \t\t\t\tconst char *reset,\n-\t\t\t\tchar sign, const char *line, int len,\n+\t\t\t\tconst char *sign, const char *line, int len,\n \t\t\t\tunsigned ws_rule, int blank_at_eof)\n {\n \tconst char *ws = NULL;\n@@ -1244,7 +1250,7 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n \t\tcontext = diff_get_color_opt(o, DIFF_CONTEXT);\n \t\treset = diff_get_color_opt(o, DIFF_RESET);\n \t\tputc('\\n', o->file);\n-\t\temit_line_0(o, context, NULL, reset, '\\\\',\n+\t\temit_line_0(o, context, NULL, reset, \"\\\\\",\n \t\t\t    nneof, strlen(nneof));\n \t\tbreak;\n \tcase DIFF_SYMBOL_SUBMODULE_HEADER:\n@@ -1282,7 +1288,7 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n \t\t\telse if (c == '-')\n \t\t\t\tset = diff_get_color_opt(o, DIFF_FILE_OLD);\n \t\t}\n-\t\temit_line_ws_markup(o, set_sign, set, reset, ' ', line, len,\n+\t\temit_line_ws_markup(o, set_sign, set, reset, \" \", line, len,\n \t\t\t\t    flags & (DIFF_SYMBOL_CONTENT_WS_MASK), 0);\n \t\tbreak;\n \tcase DIFF_SYMBOL_PLUS:\n@@ -1325,7 +1331,7 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n \t\t\t\tset = diff_get_color_opt(o, DIFF_CONTEXT_BOLD);\n \t\t\tflags &= ~DIFF_SYMBOL_CONTENT_WS_MASK;\n \t\t}\n-\t\temit_line_ws_markup(o, set_sign, set, reset, '+', line, len,\n+\t\temit_line_ws_markup(o, set_sign, set, reset, \"+\", line, len,\n \t\t\t\t    flags & DIFF_SYMBOL_CONTENT_WS_MASK,\n \t\t\t\t    flags & DIFF_SYMBOL_CONTENT_BLANK_LINE_EOF);\n \t\tbreak;\n@@ -1368,7 +1374,7 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n \t\t\telse\n \t\t\t\tset = diff_get_color_opt(o, DIFF_CONTEXT_DIM);\n \t\t}\n-\t\temit_line_ws_markup(o, set_sign, set, reset, '-', line, len,\n+\t\temit_line_ws_markup(o, set_sign, set, reset, \"-\", line, len,\n \t\t\t\t    flags & DIFF_SYMBOL_CONTENT_WS_MASK, 0);\n \t\tbreak;\n \tcase DIFF_SYMBOL_WORDS_PORCELAIN:\n-- \n2.18.0.865.gffc8e1a3cd6-goog\n\n"},{"id":"355205","messageId":"20180810224923.143625-3-sbeller@google.com","threadId":"49096","inReplyTo":"20180810224923.143625-1-sbeller@google.com","subject":"[PATCH 2/4] diff.c: add --output-indicator-{new, old, context}","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-10T22:49:21Z","receivedAt":"2018-08-10T22:49:43Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This will prove useful in range-diff in a later patch as we will be able to\ndifferentiate between adding a new file (that line is starting with +++\nand then the file name) and regular new lines.\n\nIt could also be useful for experimentation in new patch formats, i.e.\nwe could teach git to emit moved lines with lines other than +/-.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n diff.c | 21 +++++++++++++++++----\n diff.h |  5 +++++\n 2 files changed, 22 insertions(+), 4 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex b3cb73eb69a..b75eb085cb3 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1237,7 +1237,7 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n \t\t\t\t\t struct emitted_diff_symbol *eds)\n {\n \tstatic const char *nneof = \" No newline at end of file\\n\";\n-\tconst char *context, *reset, *set, *set_sign, *meta, *fraginfo;\n+\tconst char *context, *reset, *set, *set_sign, *meta, *fraginfo, *first;\n \tstruct strbuf sb = STRBUF_INIT;\n \n \tenum diff_symbol s = eds->s;\n@@ -1288,7 +1288,9 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n \t\t\telse if (c == '-')\n \t\t\t\tset = diff_get_color_opt(o, DIFF_FILE_OLD);\n \t\t}\n-\t\temit_line_ws_markup(o, set_sign, set, reset, \" \", line, len,\n+\t\tfirst = o->output_indicators[OI_CONTEXT] ?\n+\t\t\to->output_indicators[OI_CONTEXT] : \" \";\n+\t\temit_line_ws_markup(o, set_sign, set, reset, first, line, len,\n \t\t\t\t    flags & (DIFF_SYMBOL_CONTENT_WS_MASK), 0);\n \t\tbreak;\n \tcase DIFF_SYMBOL_PLUS:\n@@ -1331,7 +1333,10 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n \t\t\t\tset = diff_get_color_opt(o, DIFF_CONTEXT_BOLD);\n \t\t\tflags &= ~DIFF_SYMBOL_CONTENT_WS_MASK;\n \t\t}\n-\t\temit_line_ws_markup(o, set_sign, set, reset, \"+\", line, len,\n+\n+\t\tfirst = o->output_indicators[OI_NEW] ?\n+\t\t\to->output_indicators[OI_NEW] : \"+\";\n+\t\temit_line_ws_markup(o, set_sign, set, reset, first, line, len,\n \t\t\t\t    flags & DIFF_SYMBOL_CONTENT_WS_MASK,\n \t\t\t\t    flags & DIFF_SYMBOL_CONTENT_BLANK_LINE_EOF);\n \t\tbreak;\n@@ -1374,7 +1379,9 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n \t\t\telse\n \t\t\t\tset = diff_get_color_opt(o, DIFF_CONTEXT_DIM);\n \t\t}\n-\t\temit_line_ws_markup(o, set_sign, set, reset, \"-\", line, len,\n+\t\tfirst = o->output_indicators[OI_OLD] ?\n+\t\t\to->output_indicators[OI_OLD] : \"-\";\n+\t\temit_line_ws_markup(o, set_sign, set, reset, first, line, len,\n \t\t\t\t    flags & DIFF_SYMBOL_CONTENT_WS_MASK, 0);\n \t\tbreak;\n \tcase DIFF_SYMBOL_WORDS_PORCELAIN:\n@@ -4876,6 +4883,12 @@ int diff_opt_parse(struct diff_options *options,\n \t\t options->output_format |= DIFF_FORMAT_DIFFSTAT;\n \t} else if (!strcmp(arg, \"--no-compact-summary\"))\n \t\t options->flags.stat_with_summary = 0;\n+\telse if (skip_prefix(arg, \"--output-indicator-new=\", &arg))\n+\t\toptions->output_indicators[OI_NEW] = arg;\n+\telse if (skip_prefix(arg, \"--output-indicator-old=\", &arg))\n+\t\toptions->output_indicators[OI_OLD] = arg;\n+\telse if (skip_prefix(arg, \"--output-indicator-context=\", &arg))\n+\t\toptions->output_indicators[OI_CONTEXT] = arg;\n \n \t/* renames options */\n \telse if (starts_with(arg, \"-B\") ||\ndiff --git a/diff.h b/diff.h\nindex e1e54256c18..2d4097df1c7 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -194,6 +194,11 @@ struct diff_options {\n \tFILE *file;\n \tint close_file;\n \n+#define OI_NEW 0\n+#define OI_OLD 1\n+#define OI_CONTEXT 2\n+\tconst char *output_indicators[3];\n+\n \tstruct pathspec pathspec;\n \tpathchange_fn_t pathchange;\n \tchange_fn_t change;\n-- \n2.18.0.865.gffc8e1a3cd6-goog\n\n"},{"id":"355206","messageId":"20180810224923.143625-4-sbeller@google.com","threadId":"49096","inReplyTo":"20180810224923.143625-1-sbeller@google.com","subject":"[PATCH 3/4] range-diff: make use of different output indicators","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-10T22:49:22Z","receivedAt":"2018-08-10T22:49:44Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This change itself only changes the internal communication and should\nhave no visible effect to the user. We instruct the diff code that produces\nthe inner diffs to use X, Y, Z instead of the usual markers for new, old\nand context lines\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n range-diff.c | 15 ++++++++++++++-\n 1 file changed, 14 insertions(+), 1 deletion(-)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex b6b9abac266..6e4192c8e79 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -38,6 +38,9 @@ static int read_patches(const char *range, struct string_list *list)\n \n \targv_array_pushl(&cp.args, \"log\", \"--no-color\", \"-p\", \"--no-merges\",\n \t\t\t\"--reverse\", \"--date-order\", \"--decorate=no\",\n+\t\t\t\"--output-indicator-new=X\",\n+\t\t\t\"--output-indicator-old=Y\",\n+\t\t\t\"--output-indicator-context=Z\",\n \t\t\t\"--no-abbrev-commit\", range,\n \t\t\tNULL);\n \tcp.out = -1;\n@@ -108,8 +111,18 @@ static int read_patches(const char *range, struct string_list *list)\n \t\t\t * we are not interested.\n \t\t\t */\n \t\t\tcontinue;\n-\t\telse\n+\t\telse if (line.buf[0] == 'X') {\n+\t\t\tstrbuf_addch(&buf, '+');\n+\t\t\tstrbuf_add(&buf, line.buf + 1, line.len - 1);\n+\t\t} else if (line.buf[0] == 'Y') {\n+\t\t\tstrbuf_addch(&buf, '-');\n+\t\t\tstrbuf_add(&buf, line.buf + 1, line.len - 1);\n+\t\t} else if (line.buf[0] == 'Z') {\n+\t\t\tstrbuf_addch(&buf, ' ');\n+\t\t\tstrbuf_add(&buf, line.buf + 1, line.len - 1);\n+\t\t} else {\n \t\t\tstrbuf_addbuf(&buf, &line);\n+\t\t}\n \n \t\tstrbuf_addch(&buf, '\\n');\n \t\tutil->diffsize++;\n-- \n2.18.0.865.gffc8e1a3cd6-goog\n\n"},{"id":"355207","messageId":"20180810224923.143625-5-sbeller@google.com","threadId":"49096","inReplyTo":"20180810224923.143625-1-sbeller@google.com","subject":"[PATCH 4/4] range-diff: indent special lines as context","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-10T22:49:23Z","receivedAt":"2018-08-10T22:49:47Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The range-diff coloring is a bit fuzzy when it comes to special lines of\na diff, such as indicating new and old files with +++ and ---, as it\nwould pickup the first character and interpret it for its coloring, which\nseems annoying as in regular diffs, these lines are colored bold via\nDIFF_METAINFO.\n\nBy indenting these lines by a white space, they will be treated as context\nwhich is much more useful, an example [1] on the range diff series itself:\n\n[...]\n    + diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt\n    + new file mode 100644\n    + --- /dev/null\n    + +++ b/Documentation/git-range-diff.txt\n    +@@\n    ++git-range-diff(1)\n[...]\n    +\n      diff --git a/Makefile b/Makefile\n      --- a/Makefile\n      +++ b/Makefile\n[...]\n\nThe first lines that introduce the new file for the man page will have the\n'+' sign colored and the rest of the line will be bold.\n\nThe later lines that indicate a change to the Makefile will be treated as\ncontext both in the outer and inner diff, such that those lines stay\nregular color.\n\n[1] ./git-range-diff pr-1/dscho/branch-diff-v3...pr-1/dscho/branch-diff-v4\n    These tags are found at https://github.com/gitgitgadget/git\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n range-diff.c          |  2 ++\n t/t3206-range-diff.sh | 12 ++++++------\n 2 files changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex 6e4192c8e79..a8db7c4f8d3 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -85,6 +85,7 @@ static int read_patches(const char *range, struct string_list *list)\n \t\t\tstrbuf_addch(&buf, '\\n');\n \t\t\tif (!util->diff_offset)\n \t\t\t\tutil->diff_offset = buf.len;\n+\t\t\tstrbuf_addch(&buf, ' ');\n \t\t\tstrbuf_addbuf(&buf, &line);\n \t\t} else if (in_header) {\n \t\t\tif (starts_with(line.buf, \"Author: \")) {\n@@ -121,6 +122,7 @@ static int read_patches(const char *range, struct string_list *list)\n \t\t\tstrbuf_addch(&buf, ' ');\n \t\t\tstrbuf_add(&buf, line.buf + 1, line.len - 1);\n \t\t} else {\n+\t\t\tstrbuf_addch(&buf, ' ');\n \t\t\tstrbuf_addbuf(&buf, &line);\n \t\t}\n \ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex 7dc7c80a1de..c94f9bf5ee1 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -133,9 +133,9 @@ test_expect_success 'changed message' '\n \t    Z\n \t    +    Also a silly comment here!\n \t    +\n-\t    Zdiff --git a/file b/file\n-\t    Z--- a/file\n-\t    Z+++ b/file\n+\t    Z diff --git a/file b/file\n+\t    Z --- a/file\n+\t    Z +++ b/file\n \t3:  147e64e = 3:  b9cb956 s/11/B/\n \t4:  a63e992 = 4:  8add5f1 s/12/B/\n \tEOF\n@@ -152,9 +152,9 @@ test_expect_success 'dual-coloring' '\n \t:     <RESET>\n \t:    <REVERSE><GREEN>+<RESET><BOLD>    Also a silly comment here!<RESET>\n \t:    <REVERSE><GREEN>+<RESET>\n-\t:     diff --git a/file b/file<RESET>\n-\t:    <RED> --- a/file<RESET>\n-\t:    <GREEN> +++ b/file<RESET>\n+\t:      diff --git a/file b/file<RESET>\n+\t:      --- a/file<RESET>\n+\t:      +++ b/file<RESET>\n \t:<RED>3:  0559556 <RESET><YELLOW>!<RESET><GREEN> 3:  b9cb956<RESET><YELLOW> s/11/B/<RESET>\n \t:    <REVERSE><CYAN>@@ -10,7 +10,7 @@<RESET>\n \t:      9<RESET>\n-- \n2.18.0.865.gffc8e1a3cd6-goog\n\n"},{"id":"355347","messageId":"nycvar.QRO.7.76.6.1808131340420.71@tvgsbejvaqbjf.bet","threadId":"49096","inReplyTo":"20180810224923.143625-2-sbeller@google.com","subject":"Re: [PATCH 1/4] diff.c: emit_line_0 to take string instead of first sign","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-08-13T11:42:34Z","receivedAt":"2018-08-13T11:42:35Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn Fri, 10 Aug 2018, Stefan Beller wrote:\n\n> By providing a string as the first part of the emission we can extend\n> it later more easily.\n\nThank you for working on this!\n\n> While at it, document emit_line_0.\n> \n> [...]\n>  \n> +/*\n> + * Emits\n> + * <set_sign> <first> <reset> <set> <second> <reset> LF\n> + * if they are present. 'first' is a NULL terminated string,\n> + * 'second' is a buffer of length 'len'.\n> + */\n\nThat does not make it clear what the role of `first` or `second` is. Could\nyou clarify that?\n\n(TBH I am not so sure myself what roles they serve. Previously, it was\nkind of obvious to me that `first` tried to specify the diff marker, if\nany. But now...?)\n\nThe rest looks good to me.\n\nThanks,\nDscho\n\n>  static void emit_line_0(struct diff_options *o,\n>  \t\t\tconst char *set_sign, const char *set, const char *reset,\n> -\t\t\tint first, const char *line, int len)\n> +\t\t\tconst char *first, const char *second, int len)\n>  {\n>  \tint has_trailing_newline, has_trailing_carriage_return;\n>  \tint reverse = !!set && !!set_sign;\n> @@ -633,11 +639,11 @@ static void emit_line_0(struct diff_options *o,\n>  \n>  \tfputs(diff_line_prefix(o), file);\n>  \n> -\thas_trailing_newline = (len > 0 && line[len-1] == '\\n');\n> +\thas_trailing_newline = (len > 0 && second[len-1] == '\\n');\n>  \tif (has_trailing_newline)\n>  \t\tlen--;\n>  \n> -\thas_trailing_carriage_return = (len > 0 && line[len-1] == '\\r');\n> +\thas_trailing_carriage_return = (len > 0 && second[len-1] == '\\r');\n>  \tif (has_trailing_carriage_return)\n>  \t\tlen--;\n>  \n> @@ -655,7 +661,7 @@ static void emit_line_0(struct diff_options *o,\n>  \t}\n>  \n>  \tif (first)\n> -\t\tfputc(first, file);\n> +\t\tfputs(first, file);\n>  \n>  \tif (!len)\n>  \t\tgoto end_of_line;\n> @@ -666,7 +672,7 @@ static void emit_line_0(struct diff_options *o,\n>  \t\tfputs(set, file);\n>  \t\tneeds_reset = 1;\n>  \t}\n> -\tfwrite(line, len, 1, file);\n> +\tfwrite(second, len, 1, file);\n>  \tneeds_reset |= len > 0;\n>  \n>  end_of_line:\n> @@ -681,7 +687,7 @@ static void emit_line_0(struct diff_options *o,\n>  static void emit_line(struct diff_options *o, const char *set, const char *reset,\n>  \t\t      const char *line, int len)\n>  {\n> -\temit_line_0(o, set, NULL, reset, 0, line, len);\n> +\temit_line_0(o, set, NULL, reset, NULL, line, len);\n>  }\n>  \n>  enum diff_symbol {\n> @@ -1201,7 +1207,7 @@ static void dim_moved_lines(struct diff_options *o)\n>  static void emit_line_ws_markup(struct diff_options *o,\n>  \t\t\t\tconst char *set_sign, const char *set,\n>  \t\t\t\tconst char *reset,\n> -\t\t\t\tchar sign, const char *line, int len,\n> +\t\t\t\tconst char *sign, const char *line, int len,\n>  \t\t\t\tunsigned ws_rule, int blank_at_eof)\n>  {\n>  \tconst char *ws = NULL;\n> @@ -1244,7 +1250,7 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n>  \t\tcontext = diff_get_color_opt(o, DIFF_CONTEXT);\n>  \t\treset = diff_get_color_opt(o, DIFF_RESET);\n>  \t\tputc('\\n', o->file);\n> -\t\temit_line_0(o, context, NULL, reset, '\\\\',\n> +\t\temit_line_0(o, context, NULL, reset, \"\\\\\",\n>  \t\t\t    nneof, strlen(nneof));\n>  \t\tbreak;\n>  \tcase DIFF_SYMBOL_SUBMODULE_HEADER:\n> @@ -1282,7 +1288,7 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n>  \t\t\telse if (c == '-')\n>  \t\t\t\tset = diff_get_color_opt(o, DIFF_FILE_OLD);\n>  \t\t}\n> -\t\temit_line_ws_markup(o, set_sign, set, reset, ' ', line, len,\n> +\t\temit_line_ws_markup(o, set_sign, set, reset, \" \", line, len,\n>  \t\t\t\t    flags & (DIFF_SYMBOL_CONTENT_WS_MASK), 0);\n>  \t\tbreak;\n>  \tcase DIFF_SYMBOL_PLUS:\n> @@ -1325,7 +1331,7 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n>  \t\t\t\tset = diff_get_color_opt(o, DIFF_CONTEXT_BOLD);\n>  \t\t\tflags &= ~DIFF_SYMBOL_CONTENT_WS_MASK;\n>  \t\t}\n> -\t\temit_line_ws_markup(o, set_sign, set, reset, '+', line, len,\n> +\t\temit_line_ws_markup(o, set_sign, set, reset, \"+\", line, len,\n>  \t\t\t\t    flags & DIFF_SYMBOL_CONTENT_WS_MASK,\n>  \t\t\t\t    flags & DIFF_SYMBOL_CONTENT_BLANK_LINE_EOF);\n>  \t\tbreak;\n> @@ -1368,7 +1374,7 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n>  \t\t\telse\n>  \t\t\t\tset = diff_get_color_opt(o, DIFF_CONTEXT_DIM);\n>  \t\t}\n> -\t\temit_line_ws_markup(o, set_sign, set, reset, '-', line, len,\n> +\t\temit_line_ws_markup(o, set_sign, set, reset, \"-\", line, len,\n>  \t\t\t\t    flags & DIFF_SYMBOL_CONTENT_WS_MASK, 0);\n>  \t\tbreak;\n>  \tcase DIFF_SYMBOL_WORDS_PORCELAIN:\n> -- \n> 2.18.0.865.gffc8e1a3cd6-goog\n> \n> \n"},{"id":"355348","messageId":"nycvar.QRO.7.76.6.1808131343310.71@tvgsbejvaqbjf.bet","threadId":"49096","inReplyTo":"20180810224923.143625-3-sbeller@google.com","subject":"Re: [PATCH 2/4] diff.c: add --output-indicator-{new, old, context}","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-08-13T11:47:05Z","receivedAt":"2018-08-13T11:47:06Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Steafn,\n\nOn Fri, 10 Aug 2018, Stefan Beller wrote:\n\n> This will prove useful in range-diff in a later patch as we will be able to\n> differentiate between adding a new file (that line is starting with +++\n> and then the file name) and regular new lines.\n\nVery good!\n\n> It could also be useful for experimentation in new patch formats, i.e.\n> we could teach git to emit moved lines with lines other than +/-.\n> \n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n>  diff.c | 21 +++++++++++++++++----\n>  diff.h |  5 +++++\n>  2 files changed, 22 insertions(+), 4 deletions(-)\n> \n> diff --git a/diff.c b/diff.c\n> index b3cb73eb69a..b75eb085cb3 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -1237,7 +1237,7 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n>  \t\t\t\t\t struct emitted_diff_symbol *eds)\n>  {\n>  \tstatic const char *nneof = \" No newline at end of file\\n\";\n> -\tconst char *context, *reset, *set, *set_sign, *meta, *fraginfo;\n> +\tconst char *context, *reset, *set, *set_sign, *meta, *fraginfo, *first;\n>  \tstruct strbuf sb = STRBUF_INIT;\n>  \n>  \tenum diff_symbol s = eds->s;\n> @@ -1288,7 +1288,9 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n>  \t\t\telse if (c == '-')\n>  \t\t\t\tset = diff_get_color_opt(o, DIFF_FILE_OLD);\n>  \t\t}\n> -\t\temit_line_ws_markup(o, set_sign, set, reset, \" \", line, len,\n> +\t\tfirst = o->output_indicators[OI_CONTEXT] ?\n> +\t\t\to->output_indicators[OI_CONTEXT] : \" \";\n> +\t\temit_line_ws_markup(o, set_sign, set, reset, first, line, len,\n\nInstead of doing this over and over again, how about\n\n1) setting o->output_indicators to \" \" in diff_setup()?\n\n2) passing OI_CONTEXT to emit_line_ws_markup() instead of `first`? I.e.\n   change it to the index in the output_indicators, with -1 indicating\n   \"none\"?\n\n>  \t\t\t\t    flags & (DIFF_SYMBOL_CONTENT_WS_MASK), 0);\n>  \t\tbreak;\n>  \tcase DIFF_SYMBOL_PLUS:\n> @@ -1331,7 +1333,10 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n>  \t\t\t\tset = diff_get_color_opt(o, DIFF_CONTEXT_BOLD);\n>  \t\t\tflags &= ~DIFF_SYMBOL_CONTENT_WS_MASK;\n>  \t\t}\n> -\t\temit_line_ws_markup(o, set_sign, set, reset, \"+\", line, len,\n> +\n> +\t\tfirst = o->output_indicators[OI_NEW] ?\n> +\t\t\to->output_indicators[OI_NEW] : \"+\";\n> +\t\temit_line_ws_markup(o, set_sign, set, reset, first, line, len,\n>  \t\t\t\t    flags & DIFF_SYMBOL_CONTENT_WS_MASK,\n>  \t\t\t\t    flags & DIFF_SYMBOL_CONTENT_BLANK_LINE_EOF);\n>  \t\tbreak;\n> @@ -1374,7 +1379,9 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n>  \t\t\telse\n>  \t\t\t\tset = diff_get_color_opt(o, DIFF_CONTEXT_DIM);\n>  \t\t}\n> -\t\temit_line_ws_markup(o, set_sign, set, reset, \"-\", line, len,\n> +\t\tfirst = o->output_indicators[OI_OLD] ?\n> +\t\t\to->output_indicators[OI_OLD] : \"-\";\n> +\t\temit_line_ws_markup(o, set_sign, set, reset, first, line, len,\n>  \t\t\t\t    flags & DIFF_SYMBOL_CONTENT_WS_MASK, 0);\n>  \t\tbreak;\n>  \tcase DIFF_SYMBOL_WORDS_PORCELAIN:\n> @@ -4876,6 +4883,12 @@ int diff_opt_parse(struct diff_options *options,\n>  \t\t options->output_format |= DIFF_FORMAT_DIFFSTAT;\n>  \t} else if (!strcmp(arg, \"--no-compact-summary\"))\n>  \t\t options->flags.stat_with_summary = 0;\n> +\telse if (skip_prefix(arg, \"--output-indicator-new=\", &arg))\n> +\t\toptions->output_indicators[OI_NEW] = arg;\n> +\telse if (skip_prefix(arg, \"--output-indicator-old=\", &arg))\n> +\t\toptions->output_indicators[OI_OLD] = arg;\n> +\telse if (skip_prefix(arg, \"--output-indicator-context=\", &arg))\n> +\t\toptions->output_indicators[OI_CONTEXT] = arg;\n>  \n>  \t/* renames options */\n>  \telse if (starts_with(arg, \"-B\") ||\n> diff --git a/diff.h b/diff.h\n> index e1e54256c18..2d4097df1c7 100644\n> --- a/diff.h\n> +++ b/diff.h\n> @@ -194,6 +194,11 @@ struct diff_options {\n>  \tFILE *file;\n>  \tint close_file;\n>  \n> +#define OI_NEW 0\n> +#define OI_OLD 1\n> +#define OI_CONTEXT 2\n\nI could imagine that OI_* is too generic a prefix, and that we would want\nto have a prefix that is less prone to collide with other global\nconstants, such as OUTPUT_INDICATOR_*.\n\nCiao,\nDscho\n\n> +\tconst char *output_indicators[3];\n> +\n>  \tstruct pathspec pathspec;\n>  \tpathchange_fn_t pathchange;\n>  \tchange_fn_t change;\n> -- \n> 2.18.0.865.gffc8e1a3cd6-goog\n> \n> \n"},{"id":"355349","messageId":"nycvar.QRO.7.76.6.1808131348120.71@tvgsbejvaqbjf.bet","threadId":"49096","inReplyTo":"20180810224923.143625-4-sbeller@google.com","subject":"Re: [PATCH 3/4] range-diff: make use of different output indicators","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-08-13T11:51:35Z","receivedAt":"2018-08-13T11:51:36Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn Fri, 10 Aug 2018, Stefan Beller wrote:\n\n> This change itself only changes the internal communication and should\n> have no visible effect to the user. We instruct the diff code that produces\n> the inner diffs to use X, Y, Z instead of the usual markers for new, old\n> and context lines\n> \n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n>  range-diff.c | 15 ++++++++++++++-\n>  1 file changed, 14 insertions(+), 1 deletion(-)\n> \n> diff --git a/range-diff.c b/range-diff.c\n> index b6b9abac266..6e4192c8e79 100644\n> --- a/range-diff.c\n> +++ b/range-diff.c\n> @@ -38,6 +38,9 @@ static int read_patches(const char *range, struct string_list *list)\n>  \n>  \targv_array_pushl(&cp.args, \"log\", \"--no-color\", \"-p\", \"--no-merges\",\n>  \t\t\t\"--reverse\", \"--date-order\", \"--decorate=no\",\n> +\t\t\t\"--output-indicator-new=X\",\n> +\t\t\t\"--output-indicator-old=Y\",\n> +\t\t\t\"--output-indicator-context=Z\",\n>  \t\t\t\"--no-abbrev-commit\", range,\n>  \t\t\tNULL);\n>  \tcp.out = -1;\n> @@ -108,8 +111,18 @@ static int read_patches(const char *range, struct string_list *list)\n>  \t\t\t * we are not interested.\n>  \t\t\t */\n>  \t\t\tcontinue;\n> -\t\telse\n> +\t\telse if (line.buf[0] == 'X') {\n> +\t\t\tstrbuf_addch(&buf, '+');\n> +\t\t\tstrbuf_add(&buf, line.buf + 1, line.len - 1);\n> +\t\t} else if (line.buf[0] == 'Y') {\n> +\t\t\tstrbuf_addch(&buf, '-');\n> +\t\t\tstrbuf_add(&buf, line.buf + 1, line.len - 1);\n> +\t\t} else if (line.buf[0] == 'Z') {\n> +\t\t\tstrbuf_addch(&buf, ' ');\n> +\t\t\tstrbuf_add(&buf, line.buf + 1, line.len - 1);\n> +\t\t} else {\n>  \t\t\tstrbuf_addbuf(&buf, &line);\n> +\t\t}\n\nMy preliminary reading (I sadly lack the time to pull your branch and play\nwith it) suggests that this works, although I have to admit that X/Y/Z\nwould confuse me in 6 months from now, as they do not really read like\ndiff markers but like plain text. I could imagine that '>', '<' and '#'\nwould not impart that confusion on me.\n\nCiao,\nDscho\n\n>  \n>  \t\tstrbuf_addch(&buf, '\\n');\n>  \t\tutil->diffsize++;\n> -- \n> 2.18.0.865.gffc8e1a3cd6-goog\n> \n> \n"},{"id":"355351","messageId":"nycvar.QRO.7.76.6.1808131353280.71@tvgsbejvaqbjf.bet","threadId":"49096","inReplyTo":"20180810224923.143625-5-sbeller@google.com","subject":"Re: [PATCH 4/4] range-diff: indent special lines as context","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-08-13T11:54:25Z","receivedAt":"2018-08-13T11:54:25Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn Fri, 10 Aug 2018, Stefan Beller wrote:\n\n> The range-diff coloring is a bit fuzzy when it comes to special lines of\n> a diff, such as indicating new and old files with +++ and ---, as it\n> would pickup the first character and interpret it for its coloring, which\n> seems annoying as in regular diffs, these lines are colored bold via\n> DIFF_METAINFO.\n> \n> By indenting these lines by a white space, they will be treated as context\n> which is much more useful, an example [1] on the range diff series itself:\n> \n> [...]\n>     + diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt\n>     + new file mode 100644\n>     + --- /dev/null\n>     + +++ b/Documentation/git-range-diff.txt\n>     +@@\n>     ++git-range-diff(1)\n> [...]\n>     +\n>       diff --git a/Makefile b/Makefile\n>       --- a/Makefile\n>       +++ b/Makefile\n> [...]\n> \n> The first lines that introduce the new file for the man page will have the\n> '+' sign colored and the rest of the line will be bold.\n> \n> The later lines that indicate a change to the Makefile will be treated as\n> context both in the outer and inner diff, such that those lines stay\n> regular color.\n\nWhile I am a fan of having those lines colored correctly, I have to admit\nthat I am not exactly enthusiastic about that extra indentation...\n\nOtherwise, this looks good to me.\n\nThanks,\nDscho\n\n> \n> [1] ./git-range-diff pr-1/dscho/branch-diff-v3...pr-1/dscho/branch-diff-v4\n>     These tags are found at https://github.com/gitgitgadget/git\n> \n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n>  range-diff.c          |  2 ++\n>  t/t3206-range-diff.sh | 12 ++++++------\n>  2 files changed, 8 insertions(+), 6 deletions(-)\n> \n> diff --git a/range-diff.c b/range-diff.c\n> index 6e4192c8e79..a8db7c4f8d3 100644\n> --- a/range-diff.c\n> +++ b/range-diff.c\n> @@ -85,6 +85,7 @@ static int read_patches(const char *range, struct string_list *list)\n>  \t\t\tstrbuf_addch(&buf, '\\n');\n>  \t\t\tif (!util->diff_offset)\n>  \t\t\t\tutil->diff_offset = buf.len;\n> +\t\t\tstrbuf_addch(&buf, ' ');\n>  \t\t\tstrbuf_addbuf(&buf, &line);\n>  \t\t} else if (in_header) {\n>  \t\t\tif (starts_with(line.buf, \"Author: \")) {\n> @@ -121,6 +122,7 @@ static int read_patches(const char *range, struct string_list *list)\n>  \t\t\tstrbuf_addch(&buf, ' ');\n>  \t\t\tstrbuf_add(&buf, line.buf + 1, line.len - 1);\n>  \t\t} else {\n> +\t\t\tstrbuf_addch(&buf, ' ');\n>  \t\t\tstrbuf_addbuf(&buf, &line);\n>  \t\t}\n>  \n> diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\n> index 7dc7c80a1de..c94f9bf5ee1 100755\n> --- a/t/t3206-range-diff.sh\n> +++ b/t/t3206-range-diff.sh\n> @@ -133,9 +133,9 @@ test_expect_success 'changed message' '\n>  \t    Z\n>  \t    +    Also a silly comment here!\n>  \t    +\n> -\t    Zdiff --git a/file b/file\n> -\t    Z--- a/file\n> -\t    Z+++ b/file\n> +\t    Z diff --git a/file b/file\n> +\t    Z --- a/file\n> +\t    Z +++ b/file\n>  \t3:  147e64e = 3:  b9cb956 s/11/B/\n>  \t4:  a63e992 = 4:  8add5f1 s/12/B/\n>  \tEOF\n> @@ -152,9 +152,9 @@ test_expect_success 'dual-coloring' '\n>  \t:     <RESET>\n>  \t:    <REVERSE><GREEN>+<RESET><BOLD>    Also a silly comment here!<RESET>\n>  \t:    <REVERSE><GREEN>+<RESET>\n> -\t:     diff --git a/file b/file<RESET>\n> -\t:    <RED> --- a/file<RESET>\n> -\t:    <GREEN> +++ b/file<RESET>\n> +\t:      diff --git a/file b/file<RESET>\n> +\t:      --- a/file<RESET>\n> +\t:      +++ b/file<RESET>\n>  \t:<RED>3:  0559556 <RESET><YELLOW>!<RESET><GREEN> 3:  b9cb956<RESET><YELLOW> s/11/B/<RESET>\n>  \t:    <REVERSE><CYAN>@@ -10,7 +10,7 @@<RESET>\n>  \t:      9<RESET>\n> -- \n> 2.18.0.865.gffc8e1a3cd6-goog\n> \n> \n"},{"id":"355430","messageId":"CAGZ79kY8-GEWjK5TjkAzv1VoyLLXBD9t-B48UwPcWkogvD0MFA@mail.gmail.com","threadId":"49096","inReplyTo":"nycvar.QRO.7.76.6.1808131340420.71@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 1/4] diff.c: emit_line_0 to take string instead of first sign","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-13T18:19:23Z","receivedAt":"2018-08-13T18:19:37Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Aug 13, 2018 at 4:42 AM Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n\n\n> > +/*\n> > + * Emits\n> > + * <set_sign> <first> <reset> <set> <second> <reset> LF\n> > + * if they are present. 'first' is a NULL terminated string,\n> > + * 'second' is a buffer of length 'len'.\n> > + */\n>\n> That does not make it clear what the role of `first` or `second` is. Could\n> you clarify that?\n\nFor now it is just \"first string\" and \"second string\", where the first is\nused for signs and indicators, and the second is allowed to have '\\0'\nin it as we give the length as a parameter. This doc tried to be\nneutral w.r.t. the purpose of the first/second string.\n\n> (TBH I am not so sure myself what roles they serve. Previously, it was\n> kind of obvious to me that `first` tried to specify the diff marker, if\n> any. But now...?)\n\nOriginally I thought we'd split the line into\n\n  \"++\" \"line\"\n\nfor a range diff, but this is not the case.\n\nAs the next patch introduces configurable strings, we'd need this patch.\nI consider changing the next patch to allow only giving a single character\ninstead, such that we can keep 'first', which indicates the sign character.\n(So maybe I'd even call it 'sign').\n"},{"id":"355432","messageId":"CAGZ79kbUKoVPXq_fz7YZMOcAWEe0uiZ04Fj9KD-J4eMmwjUXTw@mail.gmail.com","threadId":"49096","inReplyTo":"nycvar.QRO.7.76.6.1808131343310.71@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 2/4] diff.c: add --output-indicator-{new, old, context}","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-13T18:23:55Z","receivedAt":"2018-08-13T18:24:09Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Aug 13, 2018 at 4:47 AM Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n>\n> Hi Steafn,\n>\n> On Fri, 10 Aug 2018, Stefan Beller wrote:\n>\n> > This will prove useful in range-diff in a later patch as we will be able to\n> > differentiate between adding a new file (that line is starting with +++\n> > and then the file name) and regular new lines.\n>\n> Very good!\n>\n> > It could also be useful for experimentation in new patch formats, i.e.\n> > we could teach git to emit moved lines with lines other than +/-.\n> >\n> > Signed-off-by: Stefan Beller <sbeller@google.com>\n> > ---\n> >  diff.c | 21 +++++++++++++++++----\n> >  diff.h |  5 +++++\n> >  2 files changed, 22 insertions(+), 4 deletions(-)\n> >\n> > diff --git a/diff.c b/diff.c\n> > index b3cb73eb69a..b75eb085cb3 100644\n> > --- a/diff.c\n> > +++ b/diff.c\n> > @@ -1237,7 +1237,7 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n> >                                        struct emitted_diff_symbol *eds)\n> >  {\n> >       static const char *nneof = \" No newline at end of file\\n\";\n> > -     const char *context, *reset, *set, *set_sign, *meta, *fraginfo;\n> > +     const char *context, *reset, *set, *set_sign, *meta, *fraginfo, *first;\n> >       struct strbuf sb = STRBUF_INIT;\n> >\n> >       enum diff_symbol s = eds->s;\n> > @@ -1288,7 +1288,9 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n> >                       else if (c == '-')\n> >                               set = diff_get_color_opt(o, DIFF_FILE_OLD);\n> >               }\n> > -             emit_line_ws_markup(o, set_sign, set, reset, \" \", line, len,\n> > +             first = o->output_indicators[OI_CONTEXT] ?\n> > +                     o->output_indicators[OI_CONTEXT] : \" \";\n> > +             emit_line_ws_markup(o, set_sign, set, reset, first, line, len,\n>\n> Instead of doing this over and over again, how about\n>\n> 1) setting o->output_indicators to \" \" in diff_setup()?\n\n... and when parsing the command line options we could already overwrite it\n  in place.\n\n> 2) passing OI_CONTEXT to emit_line_ws_markup() instead of `first`? I.e.\n>    change it to the index in the output_indicators, with -1 indicating\n>    \"none\"?\n\nThat sounds like an elegant design, as then it is super clear that 'first'\ncan only ever be a sign (or character that we chose), but\ngiving -1 for \"none\" sounds cumbersome. I'll take a look into that.\n\n> > +#define OI_NEW 0\n> > +#define OI_OLD 1\n> > +#define OI_CONTEXT 2\n>\n> I could imagine that OI_* is too generic a prefix, and that we would want\n> to have a prefix that is less prone to collide with other global\n> constants, such as OUTPUT_INDICATOR_*.\n\nI agree on that; will take the suggestion of\nOUTPUT_INDICATOR_*.\n"},{"id":"355434","messageId":"CAGZ79kainPmtHWT+11+34f4F2pGD3+too5SrG547z6HRFYjK-A@mail.gmail.com","threadId":"49096","inReplyTo":"nycvar.QRO.7.76.6.1808131348120.71@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 3/4] range-diff: make use of different output indicators","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-13T18:24:52Z","receivedAt":"2018-08-13T18:25:05Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"> >                       strbuf_addbuf(&buf, &line);\n> > +             }\n>\n> My preliminary reading (I sadly lack the time to pull your branch and play\n> with it) suggests that this works, although I have to admit that X/Y/Z\n> would confuse me in 6 months from now, as they do not really read like\n> diff markers but like plain text. I could imagine that '>', '<' and '#'\n> would not impart that confusion on me.\n\nThanks for that suggestion! (I'll change it and add a comment)\n"},{"id":"355438","messageId":"CAGZ79kYVw3AXLyB1fx07WojN3dLuxJLDrbWwC_7M=9aoB9YuCg@mail.gmail.com","threadId":"49096","inReplyTo":"nycvar.QRO.7.76.6.1808131353280.71@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 4/4] range-diff: indent special lines as context","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-13T18:36:20Z","receivedAt":"2018-08-13T18:36:34Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"> > The later lines that indicate a change to the Makefile will be treated as\n> > context both in the outer and inner diff, such that those lines stay\n> > regular color.\n>\n> While I am a fan of having those lines colored correctly, I have to admit\n> that I am not exactly enthusiastic about that extra indentation...\n>\n> Otherwise, this looks good to me.\n\nCan you explain what makes you less enthused about the indentation?\n\nAdvantage:\n* allows easy coloring (easy implementation)\nDisadvantage:\n* formats change, but the range diff is still in its early design phase,\n  so we're not breaking things, yet?\n  (Do we ever plan on sending range-diff patches that can be applied to\n   rewrite history? I am very uncertain on such a feature request.\n   It sounds cool, though)\n"},{"id":"355591","messageId":"nycvar.QRO.7.76.6.1808141713510.71@tvgsbejvaqbjf.bet","threadId":"49096","inReplyTo":"CAGZ79kYVw3AXLyB1fx07WojN3dLuxJLDrbWwC_7M=9aoB9YuCg@mail.gmail.com","subject":"Re: [PATCH 4/4] range-diff: indent special lines as context","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-08-14T18:54:24Z","receivedAt":"2018-08-14T18:54:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn Mon, 13 Aug 2018, Stefan Beller wrote:\n\n> > > The later lines that indicate a change to the Makefile will be treated as\n> > > context both in the outer and inner diff, such that those lines stay\n> > > regular color.\n> >\n> > While I am a fan of having those lines colored correctly, I have to admit\n> > that I am not exactly enthusiastic about that extra indentation...\n> >\n> > Otherwise, this looks good to me.\n> \n> Can you explain what makes you less enthused about the indentation?\n> \n> Advantage:\n> * allows easy coloring (easy implementation)\n> Disadvantage:\n> * formats change,\n\nThis is it. It breaks my visual flow.\n\n> but the range diff is still in its early design phase, so we're not\n> breaking things, yet?\n\nIndeed. We're not breaking things. If you feel strongly about it, we can\nhave that indentation, I *can* get used to it.\n\n>   (Do we ever plan on sending range-diff patches that can be applied to\n>   rewrite history? I am very uncertain on such a feature request.  It\n>   sounds cool, though)\n\nI remember that I heard you discussing this internally. I am not too big a\nfan of this idea, I have to admit. The range diff seems more designed to\nexplain how a patch series evolved, rather than providing machine-readable\ndata that allows to recreate said evolution. For example, the committer\ninformation as well as the date are missing, which would preclude a\nfaithful reconstruction.\n\nAnd that is not all: if you wanted to \"apply\" a range diff, you would need\nto know more about the base(s) of the two commit ranges. You would need to\nknow that they are at least very similar to the base onto which you want\nto apply this.\n\nAnd quite seriously, this would be the wrong way to go in my mind. We have\na very efficient data format to transport all of that information: the Git\nbundle.\n\nLet's not overload the range diff format with multiple, partially\ncontradicting purposes. Think \"separation of concerns\". It's the same\nissue, really, as trying to send highly structured data such as bug\nreports or code contributions via a medium meant to send unstructured\nplain or formatted text back and forth between human beings.\n\nCiao,\nDscho\n"},{"id":"355595","messageId":"CAGZ79kZhLsUZL+bfoovV+U6W-zX6npERRG86fS8+1GsTQmKoHA@mail.gmail.com","threadId":"49096","inReplyTo":"nycvar.QRO.7.76.6.1808141713510.71@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 4/4] range-diff: indent special lines as context","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-14T19:05:36Z","receivedAt":"2018-08-14T19:05:51Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Tue, Aug 14, 2018 at 11:54 AM Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n>\n> Hi Stefan,\n>\n> On Mon, 13 Aug 2018, Stefan Beller wrote:\n>\n> > > > The later lines that indicate a change to the Makefile will be treated as\n> > > > context both in the outer and inner diff, such that those lines stay\n> > > > regular color.\n> > >\n> > > While I am a fan of having those lines colored correctly, I have to admit\n> > > that I am not exactly enthusiastic about that extra indentation...\n> > >\n> > > Otherwise, this looks good to me.\n> >\n> > Can you explain what makes you less enthused about the indentation?\n> >\n> > Advantage:\n> > * allows easy coloring (easy implementation)\n> > Disadvantage:\n> > * formats change,\n>\n> This is it. It breaks my visual flow.\n>\n> > but the range diff is still in its early design phase, so we're not\n> > breaking things, yet?\n>\n> Indeed. We're not breaking things. If you feel strongly about it, we can\n> have that indentation, I *can* get used to it.\n\nI only feel strongly about it now as that is the *easiest* way to make\nthe colors\nlook like I want them to look. And I really value colors in the range-diff.\nEarlier you said that color-less range-diff is nearly useless for you and I\nthought it was hyperbole, but by now I realize how much truth you spoke.\nSo getting the colors fixed to not markup files (+++/ --- lines of the inner\ndiff) is a high priority for me. So high that I would compromise on the\nindentation/flow of these corner case areas.\n\n> >   (Do we ever plan on sending range-diff patches that can be applied to\n> >   rewrite history? I am very uncertain on such a feature request.  It\n> >   sounds cool, though)\n>\n> I remember that I heard you discussing this internally. I am not too big a\n> fan of this idea, I have to admit. The range diff seems more designed to\n> explain how a patch series evolved, rather than providing machine-readable\n> data that allows to recreate said evolution. For example, the committer\n> information as well as the date are missing, which would preclude a\n> faithful reconstruction.\n>\n\nAh! good point. Though we could just work around that and use the email\ndate for the new author dates. ;-)\n\n> And that is not all: if you wanted to \"apply\" a range diff, you would need\n> to know more about the base(s) of the two commit ranges. You would need to\n> know that they are at least very similar to the base onto which you want\n> to apply this.\n\nYou would say so in the cover letter \"This is a resend of sb/range-diff-colors\"\nand by the knowledge of that tip only and the range-diff you would\nknow how the new series would look like, even if it was rebased.\n\n> And quite seriously, this would be the wrong way to go in my mind. We have\n> a very efficient data format to transport all of that information: the Git\n> bundle.\n\nThe bundle format is very efficient for machine transport, but I thought the\nwhole point of the mailing list was easy human readable parts, i.e. you can\npoint out things in a diff, which you could also do in a range-diff to some\nextend. We would loose some of the \"fresh eyes\" as you'd only see the\nchanged part of the series. :-/ So yeah even for the workflow this seems\na net-negative. I just thought it would be cool.\n\n> Let's not overload the range diff format with multiple, partially\n> contradicting purposes. Think \"separation of concerns\". It's the same\n> issue, really, as trying to send highly structured data such as bug\n> reports or code contributions via a medium meant to send unstructured\n> plain or formatted text back and forth between human beings.\n\nok.\n\nThanks,\nStefan\n"},{"id":"355820","messageId":"nycvar.QRO.7.76.6.1808161022180.71@tvgsbejvaqbjf.bet","threadId":"49096","inReplyTo":"CAGZ79kZhLsUZL+bfoovV+U6W-zX6npERRG86fS8+1GsTQmKoHA@mail.gmail.com","subject":"Re: [PATCH 4/4] range-diff: indent special lines as context","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-08-16T08:22:51Z","receivedAt":"2018-08-16T08:23:00Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn Tue, 14 Aug 2018, Stefan Beller wrote:\n\n> On Tue, Aug 14, 2018 at 11:54 AM Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n> >\n> > On Mon, 13 Aug 2018, Stefan Beller wrote:\n> >\n> > > > > The later lines that indicate a change to the Makefile will be\n> > > > > treated as context both in the outer and inner diff, such that\n> > > > > those lines stay regular color.\n> > > >\n> > > > While I am a fan of having those lines colored correctly, I have\n> > > > to admit that I am not exactly enthusiastic about that extra\n> > > > indentation...\n> > > >\n> > > > Otherwise, this looks good to me.\n> > >\n> > > Can you explain what makes you less enthused about the indentation?\n> > >\n> > > Advantage:\n> > > * allows easy coloring (easy implementation)\n> > > Disadvantage:\n> > > * formats change,\n> >\n> > This is it. It breaks my visual flow.\n> >\n> > > but the range diff is still in its early design phase, so we're not\n> > > breaking things, yet?\n> >\n> > Indeed. We're not breaking things. If you feel strongly about it, we\n> > can have that indentation, I *can* get used to it.\n> \n> I only feel strongly about it now as that is the *easiest* way to make\n> the colors look like I want them to look. And I really value colors in\n> the range-diff.  Earlier you said that color-less range-diff is nearly\n> useless for you and I thought it was hyperbole, but by now I realize how\n> much truth you spoke.  So getting the colors fixed to not markup files\n> (+++/ --- lines of the inner diff) is a high priority for me. So high\n> that I would compromise on the indentation/flow of these corner case\n> areas.\n\nOkay, let's go with your indentation, then.\n\nCiao,\nDscho\n"},{"id":"355938","messageId":"20180817204354.108625-1-sbeller@google.com","threadId":"49096","inReplyTo":"nycvar.QRO.7.76.6.1808161022180.71@tvgsbejvaqbjf.bet","subject":"[PATCH 0/3] Better colors in range-diff","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-17T20:43:51Z","receivedAt":"2018-08-17T20:44:09Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This improves colors of the range-diff, see last patch for details.\nit is also available via\n\n  git fetch https://github.com/stefanbeller/git sb/range-diff-better-colors\n\nThanks,\nStefan\n\nStefan Beller (3):\n  diff.c: add --output-indicator-{new, old, context}\n  range-diff: make use of different output indicators\n  range-diff: indent special lines as context\n\n diff.c                | 21 ++++++++++++++++++---\n diff.h                |  5 +++++\n range-diff.c          | 22 +++++++++++++++++++++-\n t/t3206-range-diff.sh | 12 ++++++------\n 4 files changed, 50 insertions(+), 10 deletions(-)\n\n-- \n2.18.0.265.g16de1b435c9.dirty\n\n"},{"id":"355939","messageId":"20180817204354.108625-2-sbeller@google.com","threadId":"49096","inReplyTo":"20180817204354.108625-1-sbeller@google.com","subject":"[PATCH 1/3] diff.c: add --output-indicator-{new, old, context}","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-17T20:43:52Z","receivedAt":"2018-08-17T20:44:16Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This will prove useful in range-diff in a later patch as we will be able to\ndifferentiate between adding a new file (that line is starting with +++\nand then the file name) and regular new lines.\n\nIt could also be useful for experimentation in new patch formats, i.e.\nwe could teach git to emit moved lines with lines other than +/-.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n diff.c | 21 ++++++++++++++++++---\n diff.h |  5 +++++\n 2 files changed, 23 insertions(+), 3 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex c5c7739ce34..03486c35b75 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1281,7 +1281,9 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n \t\t\telse if (c == '-')\n \t\t\t\tset = diff_get_color_opt(o, DIFF_FILE_OLD);\n \t\t}\n-\t\temit_line_ws_markup(o, set_sign, set, reset, ' ', line, len,\n+\t\temit_line_ws_markup(o, set_sign, set, reset,\n+\t\t\t\t    o->output_indicators[OUTPUT_INDICATOR_CONTEXT],\n+\t\t\t\t    line, len,\n \t\t\t\t    flags & (DIFF_SYMBOL_CONTENT_WS_MASK), 0);\n \t\tbreak;\n \tcase DIFF_SYMBOL_PLUS:\n@@ -1324,7 +1326,9 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n \t\t\t\tset = diff_get_color_opt(o, DIFF_CONTEXT_BOLD);\n \t\t\tflags &= ~DIFF_SYMBOL_CONTENT_WS_MASK;\n \t\t}\n-\t\temit_line_ws_markup(o, set_sign, set, reset, '+', line, len,\n+\t\temit_line_ws_markup(o, set_sign, set, reset,\n+\t\t\t\t    o->output_indicators[OUTPUT_INDICATOR_NEW],\n+\t\t\t\t    line, len,\n \t\t\t\t    flags & DIFF_SYMBOL_CONTENT_WS_MASK,\n \t\t\t\t    flags & DIFF_SYMBOL_CONTENT_BLANK_LINE_EOF);\n \t\tbreak;\n@@ -1367,7 +1371,9 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n \t\t\telse\n \t\t\t\tset = diff_get_color_opt(o, DIFF_CONTEXT_DIM);\n \t\t}\n-\t\temit_line_ws_markup(o, set_sign, set, reset, '-', line, len,\n+\t\temit_line_ws_markup(o, set_sign, set, reset,\n+\t\t\t\t    o->output_indicators[OUTPUT_INDICATOR_OLD],\n+\t\t\t\t    line, len,\n \t\t\t\t    flags & DIFF_SYMBOL_CONTENT_WS_MASK, 0);\n \t\tbreak;\n \tcase DIFF_SYMBOL_WORDS_PORCELAIN:\n@@ -4382,6 +4388,9 @@ void diff_setup(struct diff_options *options)\n \n \toptions->file = stdout;\n \n+\toptions->output_indicators[OUTPUT_INDICATOR_NEW] = '+';\n+\toptions->output_indicators[OUTPUT_INDICATOR_OLD] = '-';\n+\toptions->output_indicators[OUTPUT_INDICATOR_CONTEXT] = ' ';\n \toptions->abbrev = DEFAULT_ABBREV;\n \toptions->line_termination = '\\n';\n \toptions->break_opt = -1;\n@@ -4869,6 +4878,12 @@ int diff_opt_parse(struct diff_options *options,\n \t\t options->output_format |= DIFF_FORMAT_DIFFSTAT;\n \t} else if (!strcmp(arg, \"--no-compact-summary\"))\n \t\t options->flags.stat_with_summary = 0;\n+\telse if (skip_prefix(arg, \"--output-indicator-new=\", &arg))\n+\t\toptions->output_indicators[OUTPUT_INDICATOR_NEW] = arg[0];\n+\telse if (skip_prefix(arg, \"--output-indicator-old=\", &arg))\n+\t\toptions->output_indicators[OUTPUT_INDICATOR_OLD] = arg[0];\n+\telse if (skip_prefix(arg, \"--output-indicator-context=\", &arg))\n+\t\toptions->output_indicators[OUTPUT_INDICATOR_CONTEXT] = arg[0];\n \n \t/* renames options */\n \telse if (starts_with(arg, \"-B\") ||\ndiff --git a/diff.h b/diff.h\nindex e1e54256c18..d7edc64454a 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -194,6 +194,11 @@ struct diff_options {\n \tFILE *file;\n \tint close_file;\n \n+#define OUTPUT_INDICATOR_NEW 0\n+#define OUTPUT_INDICATOR_OLD 1\n+#define OUTPUT_INDICATOR_CONTEXT 2\n+\tchar output_indicators[3];\n+\n \tstruct pathspec pathspec;\n \tpathchange_fn_t pathchange;\n \tchange_fn_t change;\n-- \n2.18.0.265.g16de1b435c9.dirty\n\n"},{"id":"355940","messageId":"20180817204354.108625-3-sbeller@google.com","threadId":"49096","inReplyTo":"20180817204354.108625-1-sbeller@google.com","subject":"[PATCH 2/3] range-diff: make use of different output indicators","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-17T20:43:53Z","receivedAt":"2018-08-17T20:44:16Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This change itself only changes the internal communication and should\nhave no visible effect to the user. We instruct the diff code that\nproduces the inner diffs to use other markers instead of the\nusual markers for new, old and context lines.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n range-diff.c | 20 +++++++++++++++++++-\n 1 file changed, 19 insertions(+), 1 deletion(-)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex b6b9abac266..a906d25f139 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -38,6 +38,14 @@ static int read_patches(const char *range, struct string_list *list)\n \n \targv_array_pushl(&cp.args, \"log\", \"--no-color\", \"-p\", \"--no-merges\",\n \t\t\t\"--reverse\", \"--date-order\", \"--decorate=no\",\n+\t\t\t/*\n+\t\t\t * Choose indicators that are not used anywhere\n+\t\t\t * else in diffs, but still look reasonable\n+\t\t\t * (e.g. will not be confusing when debugging)\n+\t\t\t */\n+\t\t\t\"--output-indicator-new=>\",\n+\t\t\t\"--output-indicator-old=<\",\n+\t\t\t\"--output-indicator-context=#\",\n \t\t\t\"--no-abbrev-commit\", range,\n \t\t\tNULL);\n \tcp.out = -1;\n@@ -108,8 +116,18 @@ static int read_patches(const char *range, struct string_list *list)\n \t\t\t * we are not interested.\n \t\t\t */\n \t\t\tcontinue;\n-\t\telse\n+\t\telse if (line.buf[0] == '>') {\n+\t\t\tstrbuf_addch(&buf, '+');\n+\t\t\tstrbuf_add(&buf, line.buf + 1, line.len - 1);\n+\t\t} else if (line.buf[0] == '<') {\n+\t\t\tstrbuf_addch(&buf, '-');\n+\t\t\tstrbuf_add(&buf, line.buf + 1, line.len - 1);\n+\t\t} else if (line.buf[0] == '#') {\n+\t\t\tstrbuf_addch(&buf, ' ');\n+\t\t\tstrbuf_add(&buf, line.buf + 1, line.len - 1);\n+\t\t} else {\n \t\t\tstrbuf_addbuf(&buf, &line);\n+\t\t}\n \n \t\tstrbuf_addch(&buf, '\\n');\n \t\tutil->diffsize++;\n-- \n2.18.0.265.g16de1b435c9.dirty\n\n"},{"id":"355941","messageId":"20180817204354.108625-4-sbeller@google.com","threadId":"49096","inReplyTo":"20180817204354.108625-1-sbeller@google.com","subject":"[PATCH 3/3] range-diff: indent special lines as context","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-17T20:43:54Z","receivedAt":"2018-08-17T20:44:16Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The range-diff coloring is a bit fuzzy when it comes to special lines of\na diff, such as indicating new and old files with +++ and ---, as it\nwould pickup the first character and interpret it for its coloring, which\nseems annoying as in regular diffs, these lines are colored bold via\nDIFF_METAINFO.\n\nBy indenting these lines by a white space, they will be treated as context\nwhich is much more useful, an example [1] on the range diff series itself:\n\n[...]\n    + diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt\n    + new file mode 100644\n    + --- /dev/null\n    + +++ b/Documentation/git-range-diff.txt\n    +@@\n    ++git-range-diff(1)\n[...]\n    +\n      diff --git a/Makefile b/Makefile\n      --- a/Makefile\n      +++ b/Makefile\n[...]\n\nThe first lines that introduce the new file for the man page will have the\n'+' sign colored and the rest of the line will be bold.\n\nThe later lines that indicate a change to the Makefile will be treated as\ncontext both in the outer and inner diff, such that those lines stay\nregular color.\n\n[1] ./git-range-diff pr-1/dscho/branch-diff-v3...pr-1/dscho/branch-diff-v4\n    These tags are found at https://github.com/gitgitgadget/git\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n range-diff.c          |  2 ++\n t/t3206-range-diff.sh | 12 ++++++------\n 2 files changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/range-diff.c b/range-diff.c\nindex a906d25f139..3e9b9844012 100644\n--- a/range-diff.c\n+++ b/range-diff.c\n@@ -90,6 +90,7 @@ static int read_patches(const char *range, struct string_list *list)\n \t\t\tstrbuf_addch(&buf, '\\n');\n \t\t\tif (!util->diff_offset)\n \t\t\t\tutil->diff_offset = buf.len;\n+\t\t\tstrbuf_addch(&buf, ' ');\n \t\t\tstrbuf_addbuf(&buf, &line);\n \t\t} else if (in_header) {\n \t\t\tif (starts_with(line.buf, \"Author: \")) {\n@@ -126,6 +127,7 @@ static int read_patches(const char *range, struct string_list *list)\n \t\t\tstrbuf_addch(&buf, ' ');\n \t\t\tstrbuf_add(&buf, line.buf + 1, line.len - 1);\n \t\t} else {\n+\t\t\tstrbuf_addch(&buf, ' ');\n \t\t\tstrbuf_addbuf(&buf, &line);\n \t\t}\n \ndiff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh\nindex 7dc7c80a1de..c94f9bf5ee1 100755\n--- a/t/t3206-range-diff.sh\n+++ b/t/t3206-range-diff.sh\n@@ -133,9 +133,9 @@ test_expect_success 'changed message' '\n \t    Z\n \t    +    Also a silly comment here!\n \t    +\n-\t    Zdiff --git a/file b/file\n-\t    Z--- a/file\n-\t    Z+++ b/file\n+\t    Z diff --git a/file b/file\n+\t    Z --- a/file\n+\t    Z +++ b/file\n \t3:  147e64e = 3:  b9cb956 s/11/B/\n \t4:  a63e992 = 4:  8add5f1 s/12/B/\n \tEOF\n@@ -152,9 +152,9 @@ test_expect_success 'dual-coloring' '\n \t:     <RESET>\n \t:    <REVERSE><GREEN>+<RESET><BOLD>    Also a silly comment here!<RESET>\n \t:    <REVERSE><GREEN>+<RESET>\n-\t:     diff --git a/file b/file<RESET>\n-\t:    <RED> --- a/file<RESET>\n-\t:    <GREEN> +++ b/file<RESET>\n+\t:      diff --git a/file b/file<RESET>\n+\t:      --- a/file<RESET>\n+\t:      +++ b/file<RESET>\n \t:<RED>3:  0559556 <RESET><YELLOW>!<RESET><GREEN> 3:  b9cb956<RESET><YELLOW> s/11/B/<RESET>\n \t:    <REVERSE><CYAN>@@ -10,7 +10,7 @@<RESET>\n \t:      9<RESET>\n-- \n2.18.0.265.g16de1b435c9.dirty\n\n"},{"id":"355950","messageId":"xmqqzhxkabv3.fsf@gitster-ct.c.googlers.com","threadId":"49096","inReplyTo":"20180817204354.108625-1-sbeller@google.com","subject":"Re: [PATCH 0/3] Better colors in range-diff","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-17T22:04:32Z","receivedAt":"2018-08-17T22:04:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> This improves colors of the range-diff, see last patch for details.\n\nHow does this relate to your other \"color with range-diff\" topic\nthat is still in flight?  This supersedes it?  Builds on it?\nSomething else?\n\nThanks.\n\n"},{"id":"355952","messageId":"CAGZ79kZdW4=Tp7ci2CbJKevxHaG4wDMzZQB2KfPQPkg1AWBkRg@mail.gmail.com","threadId":"49096","inReplyTo":"xmqqzhxkabv3.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/3] Better colors in range-diff","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-17T22:09:37Z","receivedAt":"2018-08-17T22:09:50Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Fri, Aug 17, 2018 at 3:04 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Stefan Beller <sbeller@google.com> writes:\n>\n> > This improves colors of the range-diff, see last patch for details.\n>\n> How does this relate to your other \"color with range-diff\" topic\n> that is still in flight?  This supersedes it?  Builds on it?\n> Something else?\n\nIt builds on top.\n\nSorry about missing the obvious,\nStefan\n"},{"id":"356127","messageId":"nycvar.QRO.7.76.6.1808202129420.73@tvgsbejvaqbjf.bet","threadId":"49096","inReplyTo":"20180817204354.108625-2-sbeller@google.com","subject":"Re: [PATCH 1/3] diff.c: add --output-indicator-{new, old, context}","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-08-20T19:31:43Z","receivedAt":"2018-08-20T19:31:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn Fri, 17 Aug 2018, Stefan Beller wrote:\n\n> This will prove useful in range-diff in a later patch as we will be able\n> to differentiate between adding a new file (that line is starting with\n> +++ and then the file name) and regular new lines.\n> \n> It could also be useful for experimentation in new patch formats, i.e.\n> we could teach git to emit moved lines with lines other than +/-.\n\nThanks.\n\n> diff --git a/diff.c b/diff.c\n> index c5c7739ce34..03486c35b75 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -1281,7 +1281,9 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n>  \t\t\telse if (c == '-')\n>  \t\t\t\tset = diff_get_color_opt(o, DIFF_FILE_OLD);\n>  \t\t}\n> -\t\temit_line_ws_markup(o, set_sign, set, reset, ' ', line, len,\n                                    ^\nHere we already pass `o`... so...\n\n> +\t\temit_line_ws_markup(o, set_sign, set, reset,\n> +\t\t\t\t    o->output_indicators[OUTPUT_INDICATOR_CONTEXT],\n                                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n... here, we could simply pass `OUTPUT_INDICATOR_CONTEXT` and let the\ncallee look it up in`o->output_indicators[]`...\n\nI read all three patches and did not see a reason why we could not\nsimplify the code that way.\n\nOther than that: great!\n\nThank you,\nDscho\n\n> +\t\t\t\t    line, len,\n>  \t\t\t\t    flags & (DIFF_SYMBOL_CONTENT_WS_MASK), 0);\n>  \t\tbreak;\n>  \tcase DIFF_SYMBOL_PLUS:\n> @@ -1324,7 +1326,9 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n>  \t\t\t\tset = diff_get_color_opt(o, DIFF_CONTEXT_BOLD);\n>  \t\t\tflags &= ~DIFF_SYMBOL_CONTENT_WS_MASK;\n>  \t\t}\n> -\t\temit_line_ws_markup(o, set_sign, set, reset, '+', line, len,\n> +\t\temit_line_ws_markup(o, set_sign, set, reset,\n> +\t\t\t\t    o->output_indicators[OUTPUT_INDICATOR_NEW],\n> +\t\t\t\t    line, len,\n>  \t\t\t\t    flags & DIFF_SYMBOL_CONTENT_WS_MASK,\n>  \t\t\t\t    flags & DIFF_SYMBOL_CONTENT_BLANK_LINE_EOF);\n>  \t\tbreak;\n> @@ -1367,7 +1371,9 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n>  \t\t\telse\n>  \t\t\t\tset = diff_get_color_opt(o, DIFF_CONTEXT_DIM);\n>  \t\t}\n> -\t\temit_line_ws_markup(o, set_sign, set, reset, '-', line, len,\n> +\t\temit_line_ws_markup(o, set_sign, set, reset,\n> +\t\t\t\t    o->output_indicators[OUTPUT_INDICATOR_OLD],\n> +\t\t\t\t    line, len,\n>  \t\t\t\t    flags & DIFF_SYMBOL_CONTENT_WS_MASK, 0);\n>  \t\tbreak;\n>  \tcase DIFF_SYMBOL_WORDS_PORCELAIN:\n> @@ -4382,6 +4388,9 @@ void diff_setup(struct diff_options *options)\n>  \n>  \toptions->file = stdout;\n>  \n> +\toptions->output_indicators[OUTPUT_INDICATOR_NEW] = '+';\n> +\toptions->output_indicators[OUTPUT_INDICATOR_OLD] = '-';\n> +\toptions->output_indicators[OUTPUT_INDICATOR_CONTEXT] = ' ';\n>  \toptions->abbrev = DEFAULT_ABBREV;\n>  \toptions->line_termination = '\\n';\n>  \toptions->break_opt = -1;\n> @@ -4869,6 +4878,12 @@ int diff_opt_parse(struct diff_options *options,\n>  \t\t options->output_format |= DIFF_FORMAT_DIFFSTAT;\n>  \t} else if (!strcmp(arg, \"--no-compact-summary\"))\n>  \t\t options->flags.stat_with_summary = 0;\n> +\telse if (skip_prefix(arg, \"--output-indicator-new=\", &arg))\n> +\t\toptions->output_indicators[OUTPUT_INDICATOR_NEW] = arg[0];\n> +\telse if (skip_prefix(arg, \"--output-indicator-old=\", &arg))\n> +\t\toptions->output_indicators[OUTPUT_INDICATOR_OLD] = arg[0];\n> +\telse if (skip_prefix(arg, \"--output-indicator-context=\", &arg))\n> +\t\toptions->output_indicators[OUTPUT_INDICATOR_CONTEXT] = arg[0];\n>  \n>  \t/* renames options */\n>  \telse if (starts_with(arg, \"-B\") ||\n> diff --git a/diff.h b/diff.h\n> index e1e54256c18..d7edc64454a 100644\n> --- a/diff.h\n> +++ b/diff.h\n> @@ -194,6 +194,11 @@ struct diff_options {\n>  \tFILE *file;\n>  \tint close_file;\n>  \n> +#define OUTPUT_INDICATOR_NEW 0\n> +#define OUTPUT_INDICATOR_OLD 1\n> +#define OUTPUT_INDICATOR_CONTEXT 2\n> +\tchar output_indicators[3];\n> +\n>  \tstruct pathspec pathspec;\n>  \tpathchange_fn_t pathchange;\n>  \tchange_fn_t change;\n> -- \n> 2.18.0.265.g16de1b435c9.dirty\n> \n> \n"},{"id":"356130","messageId":"CAGZ79kY8oadQ2nKWr7znMa8QJgLcidb0FefPDBaba+q2J=nfPg@mail.gmail.com","threadId":"49096","inReplyTo":"nycvar.QRO.7.76.6.1808202129420.73@tvgsbejvaqbjf.bet","subject":"Re: [PATCH 1/3] diff.c: add --output-indicator-{new, old, context}","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-20T19:39:59Z","receivedAt":"2018-08-20T19:40:14Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Aug 20, 2018 at 12:31 PM Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n>\n> Hi Stefan,\n>\n> On Fri, 17 Aug 2018, Stefan Beller wrote:\n>\n> > This will prove useful in range-diff in a later patch as we will be able\n> > to differentiate between adding a new file (that line is starting with\n> > +++ and then the file name) and regular new lines.\n> >\n> > It could also be useful for experimentation in new patch formats, i.e.\n> > we could teach git to emit moved lines with lines other than +/-.\n>\n> Thanks.\n>\n> > diff --git a/diff.c b/diff.c\n> > index c5c7739ce34..03486c35b75 100644\n> > --- a/diff.c\n> > +++ b/diff.c\n> > @@ -1281,7 +1281,9 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n> >                       else if (c == '-')\n> >                               set = diff_get_color_opt(o, DIFF_FILE_OLD);\n> >               }\n> > -             emit_line_ws_markup(o, set_sign, set, reset, ' ', line, len,\n>                                     ^\n> Here we already pass `o`... so...\n>\n> > +             emit_line_ws_markup(o, set_sign, set, reset,\n> > +                                 o->output_indicators[OUTPUT_INDICATOR_CONTEXT],\n>                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n> ... here, we could simply pass `OUTPUT_INDICATOR_CONTEXT` and let the\n> callee look it up in`o->output_indicators[]`...\n>\n> I read all three patches and did not see a reason why we could not\n> simplify the code that way.\n>\n> Other than that: great!\n\nThanks!\n\nI considered it, but was put off by the (small) effort of yet another\ndiff refactoring.\n\nI'll include it in a resend if a resend is needed, otherwise\nI would suggest to make it a patch on top?\n\nThanks,\nStefan\n"},{"id":"356176","messageId":"nycvar.QRO.7.76.6.1808211813020.73@tvgsbejvaqbjf.bet","threadId":"49096","inReplyTo":"CAGZ79kY8oadQ2nKWr7znMa8QJgLcidb0FefPDBaba+q2J=nfPg@mail.gmail.com","subject":"Re: [PATCH 1/3] diff.c: add --output-indicator-{new, old, context}","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-08-21T16:13:23Z","receivedAt":"2018-08-21T16:13:32Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn Mon, 20 Aug 2018, Stefan Beller wrote:\n\n> On Mon, Aug 20, 2018 at 12:31 PM Johannes Schindelin\n> <Johannes.Schindelin@gmx.de> wrote:\n> >\n> > On Fri, 17 Aug 2018, Stefan Beller wrote:\n> >\n> > > This will prove useful in range-diff in a later patch as we will be able\n> > > to differentiate between adding a new file (that line is starting with\n> > > +++ and then the file name) and regular new lines.\n> > >\n> > > It could also be useful for experimentation in new patch formats, i.e.\n> > > we could teach git to emit moved lines with lines other than +/-.\n> >\n> > Thanks.\n> >\n> > > diff --git a/diff.c b/diff.c\n> > > index c5c7739ce34..03486c35b75 100644\n> > > --- a/diff.c\n> > > +++ b/diff.c\n> > > @@ -1281,7 +1281,9 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n> > >                       else if (c == '-')\n> > >                               set = diff_get_color_opt(o, DIFF_FILE_OLD);\n> > >               }\n> > > -             emit_line_ws_markup(o, set_sign, set, reset, ' ', line, len,\n> >                                     ^\n> > Here we already pass `o`... so...\n> >\n> > > +             emit_line_ws_markup(o, set_sign, set, reset,\n> > > +                                 o->output_indicators[OUTPUT_INDICATOR_CONTEXT],\n> >                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n> > ... here, we could simply pass `OUTPUT_INDICATOR_CONTEXT` and let the\n> > callee look it up in`o->output_indicators[]`...\n> >\n> > I read all three patches and did not see a reason why we could not\n> > simplify the code that way.\n> >\n> > Other than that: great!\n> \n> Thanks!\n> \n> I considered it, but was put off by the (small) effort of yet another\n> diff refactoring.\n> \n> I'll include it in a resend if a resend is needed, otherwise\n> I would suggest to make it a patch on top?\n\nSounds good!\nDscho\n"},{"id":"356316","messageId":"20180822222548.236699-1-sbeller@google.com","threadId":"49096","inReplyTo":"nycvar.QRO.7.76.6.1808211813020.73@tvgsbejvaqbjf.bet","subject":"[PATCH] diff.c: pass sign_index to emit_line_ws_markup","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-22T22:25:48Z","receivedAt":"2018-08-22T22:33:23Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Instead of passing the sign directly to emit_line_ws_markup, pass only the\nindex to lookup the sign in diff_options->output_indicators.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n diff.c | 12 +++++-------\n 1 file changed, 5 insertions(+), 7 deletions(-)\n \nSo something like this on top of sb/range-diff-colors ?\nIf a resend is needed I'll squash this in (or carry it as a cleanup patch early\nin the series), otherwise we could put this on top.\n\nThanks,\nStefan\n \n\ndiff --git a/diff.c b/diff.c\nindex 03486c35b75..17e33d506a1 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1199,10 +1199,11 @@ static void dim_moved_lines(struct diff_options *o)\n static void emit_line_ws_markup(struct diff_options *o,\n \t\t\t\tconst char *set_sign, const char *set,\n \t\t\t\tconst char *reset,\n-\t\t\t\tchar sign, const char *line, int len,\n+\t\t\t\tint sign_index, const char *line, int len,\n \t\t\t\tunsigned ws_rule, int blank_at_eof)\n {\n \tconst char *ws = NULL;\n+\tint sign = o->output_indicators[sign_index];\n \n \tif (o->ws_error_highlight & ws_rule) {\n \t\tws = diff_get_color_opt(o, DIFF_WHITESPACE);\n@@ -1282,8 +1283,7 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n \t\t\t\tset = diff_get_color_opt(o, DIFF_FILE_OLD);\n \t\t}\n \t\temit_line_ws_markup(o, set_sign, set, reset,\n-\t\t\t\t    o->output_indicators[OUTPUT_INDICATOR_CONTEXT],\n-\t\t\t\t    line, len,\n+\t\t\t\t    OUTPUT_INDICATOR_CONTEXT, line, len,\n \t\t\t\t    flags & (DIFF_SYMBOL_CONTENT_WS_MASK), 0);\n \t\tbreak;\n \tcase DIFF_SYMBOL_PLUS:\n@@ -1327,8 +1327,7 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n \t\t\tflags &= ~DIFF_SYMBOL_CONTENT_WS_MASK;\n \t\t}\n \t\temit_line_ws_markup(o, set_sign, set, reset,\n-\t\t\t\t    o->output_indicators[OUTPUT_INDICATOR_NEW],\n-\t\t\t\t    line, len,\n+\t\t\t\t    OUTPUT_INDICATOR_NEW, line, len,\n \t\t\t\t    flags & DIFF_SYMBOL_CONTENT_WS_MASK,\n \t\t\t\t    flags & DIFF_SYMBOL_CONTENT_BLANK_LINE_EOF);\n \t\tbreak;\n@@ -1372,8 +1371,7 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n \t\t\t\tset = diff_get_color_opt(o, DIFF_CONTEXT_DIM);\n \t\t}\n \t\temit_line_ws_markup(o, set_sign, set, reset,\n-\t\t\t\t    o->output_indicators[OUTPUT_INDICATOR_OLD],\n-\t\t\t\t    line, len,\n+\t\t\t\t    OUTPUT_INDICATOR_OLD, line, len,\n \t\t\t\t    flags & DIFF_SYMBOL_CONTENT_WS_MASK, 0);\n \t\tbreak;\n \tcase DIFF_SYMBOL_WORDS_PORCELAIN:\n-- \n2.18.0.265.g16de1b435c9.dirty\n\n"},{"id":"356362","messageId":"nycvar.QRO.7.76.6.1808231625510.73@tvgsbejvaqbjf.bet","threadId":"49096","inReplyTo":"20180822222548.236699-1-sbeller@google.com","subject":"Re: [PATCH] diff.c: pass sign_index to emit_line_ws_markup","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-08-23T14:26:14Z","receivedAt":"2018-08-23T14:26:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Stefan,\n\nOn Wed, 22 Aug 2018, Stefan Beller wrote:\n\n> Instead of passing the sign directly to emit_line_ws_markup, pass only the\n> index to lookup the sign in diff_options->output_indicators.\n> \n> Signed-off-by: Stefan Beller <sbeller@google.com>\n\nLooks good to me!\n\n> ---\n>  diff.c | 12 +++++-------\n>  1 file changed, 5 insertions(+), 7 deletions(-)\n>  \n> So something like this on top of sb/range-diff-colors ?  If a resend is\n> needed I'll squash this in (or carry it as a cleanup patch early in the\n> series), otherwise we could put this on top.\n\nI'd leave this as a separate commit.\n\nCiao,\nDscho\n\n> \n> Thanks,\n> Stefan\n>  \n> \n> diff --git a/diff.c b/diff.c\n> index 03486c35b75..17e33d506a1 100644\n> --- a/diff.c\n> +++ b/diff.c\n> @@ -1199,10 +1199,11 @@ static void dim_moved_lines(struct diff_options *o)\n>  static void emit_line_ws_markup(struct diff_options *o,\n>  \t\t\t\tconst char *set_sign, const char *set,\n>  \t\t\t\tconst char *reset,\n> -\t\t\t\tchar sign, const char *line, int len,\n> +\t\t\t\tint sign_index, const char *line, int len,\n>  \t\t\t\tunsigned ws_rule, int blank_at_eof)\n>  {\n>  \tconst char *ws = NULL;\n> +\tint sign = o->output_indicators[sign_index];\n>  \n>  \tif (o->ws_error_highlight & ws_rule) {\n>  \t\tws = diff_get_color_opt(o, DIFF_WHITESPACE);\n> @@ -1282,8 +1283,7 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n>  \t\t\t\tset = diff_get_color_opt(o, DIFF_FILE_OLD);\n>  \t\t}\n>  \t\temit_line_ws_markup(o, set_sign, set, reset,\n> -\t\t\t\t    o->output_indicators[OUTPUT_INDICATOR_CONTEXT],\n> -\t\t\t\t    line, len,\n> +\t\t\t\t    OUTPUT_INDICATOR_CONTEXT, line, len,\n>  \t\t\t\t    flags & (DIFF_SYMBOL_CONTENT_WS_MASK), 0);\n>  \t\tbreak;\n>  \tcase DIFF_SYMBOL_PLUS:\n> @@ -1327,8 +1327,7 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n>  \t\t\tflags &= ~DIFF_SYMBOL_CONTENT_WS_MASK;\n>  \t\t}\n>  \t\temit_line_ws_markup(o, set_sign, set, reset,\n> -\t\t\t\t    o->output_indicators[OUTPUT_INDICATOR_NEW],\n> -\t\t\t\t    line, len,\n> +\t\t\t\t    OUTPUT_INDICATOR_NEW, line, len,\n>  \t\t\t\t    flags & DIFF_SYMBOL_CONTENT_WS_MASK,\n>  \t\t\t\t    flags & DIFF_SYMBOL_CONTENT_BLANK_LINE_EOF);\n>  \t\tbreak;\n> @@ -1372,8 +1371,7 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,\n>  \t\t\t\tset = diff_get_color_opt(o, DIFF_CONTEXT_DIM);\n>  \t\t}\n>  \t\temit_line_ws_markup(o, set_sign, set, reset,\n> -\t\t\t\t    o->output_indicators[OUTPUT_INDICATOR_OLD],\n> -\t\t\t\t    line, len,\n> +\t\t\t\t    OUTPUT_INDICATOR_OLD, line, len,\n>  \t\t\t\t    flags & DIFF_SYMBOL_CONTENT_WS_MASK, 0);\n>  \t\tbreak;\n>  \tcase DIFF_SYMBOL_WORDS_PORCELAIN:\n> -- \n> 2.18.0.265.g16de1b435c9.dirty\n> \n> \n"}]}