{"thread":{"id":"49974","subject":"Difficulty with parsing colorized diff output","startedAt":"2018-12-08T00:10:08Z","lastAt":"2018-12-12T13:52:58Z","messageCount":10,"participants":["George King","Jeff King","Stefan Beller","Ævar Arnfjörð Bjarmason"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"364779","messageId":"799879BD-A2F0-487C-AA05-8054AC62C5BD@gmail.com","threadId":"49974","inReplyTo":null,"subject":"Difficulty with parsing colorized diff output","fromName":"George King","fromEmail":"george.w.king@gmail.com","sentAt":"2018-12-08T00:09:58Z","receivedAt":"2018-12-08T00:10:08Z","isPatch":false,"sender":{"key":"george.w.king@gmail.com","avatar":"https://gravatar.com/avatar/9f94a4a17188bfe407b5d04a8d2563760028f802a0536e61c076dcb806f23226?d=mp&s=160"},"body":"Hello, I have a rather elaborate diff highlighter that I have implemented as a post-processor to regular git output. I am writing to discuss some difficult aspects of git diff's color output that I am observing with version 2.19.2. This is not a regression report; I am trying to implement a new feature and am stymied by these details.\n\nMy goal is to detect SGR color sequences, e.g. '\\x1b[32m', that exist in the source text, and have my highlighter print escaped representations of those. For example, I have checked in files that are expected test outputs for tools that emit color codes, and diffs of those get very confusing.\n\nFiguring out which color codes are from the source text and which were added by git is proving very difficult. The obvious solution is to turn git diff coloring off, but as far as I can see this also turns off all coloring for logs, which is undesirable.\n\nThen I tried to remove just the color codes that git adds to the diff. This almost works, but there are some irregularities. Most lines begin with a style/color code and end with a reset code, which would be a perfect indicator that git is using colors. However:\n\n* Context lines do not begin with reset code, but do end with a reset code. It would be preferable in my opinion if they had both (like every other line), or none at all.\n\n* Added lines have excess codes after the plus sign. The entire prefix is, `\\x1b[32m+\\x1b[m\\x1b[32m` translating to GREEN PLUS RESET GREEN. Emitting codes after the plus sign makes the parsing more complex and idiosyncratic.\n\n\nIn summary, I would like to suggest the following improvements:\n\n* Remove the excess codes after the plus sign.\n\n* When git diff is adding colors, ensure that every line begins with an SGR code and ends with the RESET code.\n\n* Add a config feature to turn on log coloring while leaving diff coloring off.\n\n\nI would be willing to attempt a fix for this myself, but I'd like to hear what the maintainers think first, and would appreciate any hints as to where I should start looking in the code base.\n\n\nIf anyone is curious about the implementation it is called `same-same` and lives here: https://github.com/gwk/pithy/blob/master/pithy/bin/same_same.py\n\nI configure it like this in .gitconfig:\n\n[core]\n  pager = same-same | LESSANSIENDCHARS=mK less --RAW-CONTROL-CHARS\n[interactive]\n  diffFilter = same-same -interactive | LESSANSIENDCHARS=mK less --RAW-CONTROL-CHARS\n\n\nThank you,\nGeorge\n\n"},{"id":"364797","messageId":"20181208071634.GA18272@sigill.intra.peff.net","threadId":"49974","inReplyTo":"799879BD-A2F0-487C-AA05-8054AC62C5BD@gmail.com","subject":"Re: Difficulty with parsing colorized diff output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-12-08T07:16:34Z","receivedAt":"2018-12-08T07:16:38Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 07, 2018 at 07:09:58PM -0500, George King wrote:\n\n> My goal is to detect SGR color sequences, e.g. '\\x1b[32m', that exist\n> in the source text, and have my highlighter print escaped\n> representations of those. For example, I have checked in files that\n> are expected test outputs for tools that emit color codes, and diffs\n> of those get very confusing.\n> \n> Figuring out which color codes are from the source text and which were\n> added by git is proving very difficult. The obvious solution is to\n> turn git diff coloring off, but as far as I can see this also turns\n> off all coloring for logs, which is undesirable.\n\nAnother gotcha that I don't think you've hit yet: whitespace\nhighlighting can color arbitrary parts of the line.\n\nTry this, for example:\n\n  printf 'foo\\n' >old\n  printf '\\t \\tfoo\\n' >new\n  git diff --no-index old new\n\nSo I'm not sure what you want to do can ever be completely robust. That\nsaid, I'm not opposed to making Git's color output more regular. It\nseems like a reasonable cleanup on its own.\n\n(For the Git project itself, we long ago realized that putting raw color\ncodes into the source is a big pain when working with diffs, and we\ninstead use tools like t/test-lib-functions.sh's test_decode_color).\n\n> * Context lines do not begin with reset code, but do end with a reset\n> code. It would be preferable in my opinion if they had both (like\n> every other line), or none at all.\n\nContext lines do have both. It's just that the default color for context\nlines is empty. ;)\n\nBut yes, I think it might be reasonable to recognize when an opening\ncolor is empty, and turn the closing reset into a noop in that case (or\nI guess you are also advocating the opposite: turn an empty color into a\nnoop \\x1b[m or similar).\n\nI think most of the coloring, including context lines, is coming from\ndiff.c:emit_diff_symbol_from_struct(). Instead of unconditionally\ncalling:\n\n  context = diff_get_color_opt(o, DIFF_CONTEXT);\n  reset = diff_get_color_opt(o, DIFF_RESET);\n\nI suspect we could have a helper like this:\n\n  static const char *diff_get_reset(const char *color)\n  {\n\tif (!color || !*color)\n\t\treturn \"\";\n\treturn diff_colors[DIFF_RESET];\n  }\n  ...\n  context = diff_get_color_opt(o, DIFF_CONTEXT);\n  reset = diff_get_reset(context);\n\n> * Added lines have excess codes after the plus sign. The entire prefix\n> is, `\\x1b[32m+\\x1b[m\\x1b[32m` translating to GREEN PLUS RESET GREEN.\n> Emitting codes after the plus sign makes the parsing more complex and\n> idiosyncratic.\n\nYeah, I've run into this one, too (when working on diff-highlight, which\nsimilarly tries to pass-through Git's existing colors).\n\nIt's unfortunately not quite trivial, due to some interactions with\nwhitespace coloring. See this old patch:\n\n  https://public-inbox.org/git/20101109220136.GA5617@sigill.intra.peff.net/\n\nand the followup:\n\n  https://public-inbox.org/git/20101118064050.GA12825@sigill.intra.peff.net/\n\n> * Add a config feature to turn on log coloring while leaving diff coloring off.\n\nYes, the fact that --pretty log coloring is tied to diffs is mostly a\nhistorical artifact. I think it would be OK to introduce a color.log\nconfig option that defaults to the value of color.diff if unset. That\nwould be backwards compatible, but allow you to set them independently\nif you wanted to.\n\n-Peff\n"},{"id":"365020","messageId":"CAGZ79kbd=2_eHdbVYwmNoAYupwnP3YDn6nT0m=v1CL0AkWXk=Q@mail.gmail.com","threadId":"49974","inReplyTo":"20181208071634.GA18272@sigill.intra.peff.net","subject":"Re: Difficulty with parsing colorized diff output","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-12-11T03:26:46Z","receivedAt":"2018-12-11T03:27:00Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"> (For the Git project itself, we long ago realized that putting raw color\n> codes into the source is a big pain when working with diffs, and we\n> instead use tools like t/test-lib-functions.sh's test_decode_color).\n\nAnd also we hid the colors behind #defines and such.\n\n> > * Context lines do not begin with reset code, but do end with a reset\n> > code. It would be preferable in my opinion if they had both (like\n> > every other line), or none at all.\n>\n> Context lines do have both. It's just that the default color for context\n> lines is empty. ;)\n\nThe content itself can contain color codes.\n\nInstead of unconditionally resetting each line, we could parse each\ncontent line to determine if we actually have to reset the colors.\n\n>\n> But yes, I think it might be reasonable to recognize when an opening\n> color is empty, and turn the closing reset into a noop in that case (or\n> I guess you are also advocating the opposite: turn an empty color into a\n> noop \\x1b[m or similar).\n\n\n>\n> I think most of the coloring, including context lines, is coming from\n> diff.c:emit_diff_symbol_from_struct(). Instead of unconditionally\n> calling:\n>\n>   context = diff_get_color_opt(o, DIFF_CONTEXT);\n>   reset = diff_get_color_opt(o, DIFF_RESET);\n>\n> I suspect we could have a helper like this:\n>\n>   static const char *diff_get_reset(const char *color)\n>   {\n>         if (!color || !*color)\n>                 return \"\";\n>         return diff_colors[DIFF_RESET];\n>   }\n>   ...\n>   context = diff_get_color_opt(o, DIFF_CONTEXT);\n>   reset = diff_get_reset(context);\n\nAnother easier way to do so would be to drop\nthe line\n\n    needs_reset = 1; /* 'line' may contain color codes. */\n\nin diff.c::emit_line_0\nI run the test suite and it passes (I thought we had a test\nenforcing we'd reset any user provided coloring).\n\n> > * Added lines have excess codes after the plus sign. The entire prefix\n> > is, `\\x1b[32m+\\x1b[m\\x1b[32m` translating to GREEN PLUS RESET GREEN.\n> > Emitting codes after the plus sign makes the parsing more complex and\n> > idiosyncratic.\n\nThen we have broken code in diff.c::emit_line_ws_markup\nin the last case (\"else {\") which first emits the sign via\nemit_line_0 and then the rest via ws_check_emit.\nIt sounds like we'd want to replace `reset` in the call\nof emit_line_0 with\n  set == set_sign ? NULL : reset\nand set in the call to ws_check_emit with\n  set == set_sign ? NULL : set\n\nManually looking at some diff output  of said diff we'd get\n  <RED>- emit_line_0(o, set_sign ? set_sign : set, NULL, !!set_sign,\nreset,<RESET>\n  <GREEN>+<RESET> <GREEN>emit_line_0(o, set_sign ? set_sign : set,\nNULL, !!set_sign, set == set_sign ? NULL : reset,<RESET>\n\nand the issue is that we do not color the beginning white space\nof the emission from ws_check_emit but maybe we should.\n\nAnother idea would be to allow Git to output its output\nas if it was run through test_decode_color, slightly related:\nhttps://public-inbox.org/git/20180804015317.182683-8-sbeller@google.com/\ni.e. we'd markup the output instead of coloring it.\n"},{"id":"365053","messageId":"20181211101742.GE31588@sigill.intra.peff.net","threadId":"49974","inReplyTo":"CAGZ79kbd=2_eHdbVYwmNoAYupwnP3YDn6nT0m=v1CL0AkWXk=Q@mail.gmail.com","subject":"Re: Difficulty with parsing colorized diff output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-12-11T10:17:42Z","receivedAt":"2018-12-11T10:18:08Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Dec 10, 2018 at 07:26:46PM -0800, Stefan Beller wrote:\n\n> > Context lines do have both. It's just that the default color for context\n> > lines is empty. ;)\n> \n> The content itself can contain color codes.\n> \n> Instead of unconditionally resetting each line, we could parse each\n> content line to determine if we actually have to reset the colors.\n\nGood point. I don't recall that being the motivation back when this\nbehavior started, but it's a nice side effect (and the more recent line\nyou mentioned in emit_line_0 certainly is doing it intentionally).\n\nThat doesn't cover _other_ terminal codes, which could also make for\nconfusing output, but I do think color codes are somewhat special. We\ngenerally send patches through \"less -R\", which will pass through the\ncolors but show escaped versions of other codes.\n\n> Another idea would be to allow Git to output its output\n> as if it was run through test_decode_color, slightly related:\n> https://public-inbox.org/git/20180804015317.182683-8-sbeller@google.com/\n> i.e. we'd markup the output instead of coloring it.\n\nYeah, I think in the most general form, the problem is that colorizing\n(including whitespace highlighting) loses information within a single\nline. It would be nice to have a machine-readable format that represents\nall the various annotations (like whitespace and coloring moved bits)\nthat Git computes.\n\n-Peff\n"},{"id":"365081","messageId":"492187CE-2AC0-49CE-A09D-ABD4A51BE4C6@gmail.com","threadId":"49974","inReplyTo":"20181211101742.GE31588@sigill.intra.peff.net","subject":"Re: Difficulty with parsing colorized diff output","fromName":"George King","fromEmail":"george.w.king@gmail.com","sentAt":"2018-12-11T14:47:17Z","receivedAt":"2018-12-11T14:47:28Z","isPatch":false,"sender":{"key":"george.w.king@gmail.com","avatar":"https://gravatar.com/avatar/9f94a4a17188bfe407b5d04a8d2563760028f802a0536e61c076dcb806f23226?d=mp&s=160"},"body":"Peff & Stefan, thank you for the feedback. For my purposes, I am content to rely on gitconfig to reduce the colors to something that I can parse without losing information. Since my first email I have found that `wsErrorHighlight = none` gets rid of the problematic extra green highlights in the `+` lines.\n\nI still think that a config to differentiate log coloring from diff coloring would be worthwhile, as it would guarantee that highlighters are getting unadulterated lines from git.\n\nI also think that bracketing every colored line with a color code before the space/plus/minus and a reset just before the newline would be smart, because then a parser can just parse line by line: if there is an SGR sequence before the space/plus/minus, then it would know to strip off the final reset. This is in contrast to how it stands now, where a context line (with no leading color) is ambiguous by itself; I have to remember from previous lines in the hunk that we have been seeing colors in order to know wether I should strip off the reset.\n\nI agree that a machine-readable format would be nice, but regardless it would be useful to make the regular output more parser-friendly.\n\n\n> On 2018-12-11, at 5:17 AM, Jeff King <peff@peff.net> wrote:\n> \n> On Mon, Dec 10, 2018 at 07:26:46PM -0800, Stefan Beller wrote:\n> \n>>> Context lines do have both. It's just that the default color for context\n>>> lines is empty. ;)\n>> \n>> The content itself can contain color codes.\n>> \n>> Instead of unconditionally resetting each line, we could parse each\n>> content line to determine if we actually have to reset the colors.\n> \n> Good point. I don't recall that being the motivation back when this\n> behavior started, but it's a nice side effect (and the more recent line\n> you mentioned in emit_line_0 certainly is doing it intentionally).\n> \n> That doesn't cover _other_ terminal codes, which could also make for\n> confusing output, but I do think color codes are somewhat special. We\n> generally send patches through \"less -R\", which will pass through the\n> colors but show escaped versions of other codes.\n> \n>> Another idea would be to allow Git to output its output\n>> as if it was run through test_decode_color, slightly related:\n>> https://public-inbox.org/git/20180804015317.182683-8-sbeller@google.com/\n>> i.e. we'd markup the output instead of coloring it.\n> \n> Yeah, I think in the most general form, the problem is that colorizing\n> (including whitespace highlighting) loses information within a single\n> line. It would be nice to have a machine-readable format that represents\n> all the various annotations (like whitespace and coloring moved bits)\n> that Git computes.\n> \n> -Peff\n\n"},{"id":"365099","messageId":"871s6oni3a.fsf@evledraar.gmail.com","threadId":"49974","inReplyTo":"20181211101742.GE31588@sigill.intra.peff.net","subject":"Re: Difficulty with parsing colorized diff output","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-12-11T16:28:25Z","receivedAt":"2018-12-11T16:28:30Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Dec 11 2018, Jeff King wrote:\n\n> On Mon, Dec 10, 2018 at 07:26:46PM -0800, Stefan Beller wrote:\n>\n>> > Context lines do have both. It's just that the default color for context\n>> > lines is empty. ;)\n>>\n>> The content itself can contain color codes.\n>>\n>> Instead of unconditionally resetting each line, we could parse each\n>> content line to determine if we actually have to reset the colors.\n>\n> Good point. I don't recall that being the motivation back when this\n> behavior started, but it's a nice side effect (and the more recent line\n> you mentioned in emit_line_0 certainly is doing it intentionally).\n>\n> That doesn't cover _other_ terminal codes, which could also make for\n> confusing output, but I do think color codes are somewhat special. We\n> generally send patches through \"less -R\", which will pass through the\n> colors but show escaped versions of other codes.\n\nI wonder if optimizing this one way or the other matters for some\nterminals. I.e. if we print out some huge diff of thousands of\nconsecutive \"green\" added lines is it faster/slower on some of them to\ndo one \"begin green\" and \"reset\" at the end, or is one line at a time\nbetter, or doesn't it matter at all?\n"},{"id":"365100","messageId":"EB1AF739-F97B-4905-9736-2A003722AD9A@gmail.com","threadId":"49974","inReplyTo":"871s6oni3a.fsf@evledraar.gmail.com","subject":"Re: Difficulty with parsing colorized diff output","fromName":"George King","fromEmail":"george.w.king@gmail.com","sentAt":"2018-12-11T16:41:18Z","receivedAt":"2018-12-11T16:41:29Z","isPatch":false,"sender":{"key":"george.w.king@gmail.com","avatar":"https://gravatar.com/avatar/9f94a4a17188bfe407b5d04a8d2563760028f802a0536e61c076dcb806f23226?d=mp&s=160"},"body":"I first started playing around with terminal colors about 5 years ago, and I recall learning the hard way that Apple Terminal at least behaves very strangely when you have background colors cross line boundaries: background colors disappeared when I scrolled lines back into view. I filed a bug thinking it couldn't be right and Apple closed it as behaving according to compatibility expectations. I never figured out whether they had misunderstood my report or if old terminals were just that crazy. Instead I decided that the safe thing to do was reset after every line. Perhaps some git author reached the same conclusion.\n\nFrom the perspective of parsing this output, it is really much easier if each line can be understood without considering state of previous lines. If anything, I think it is a safe approach to ensuring that it renders correctly on various terminals as well.\n\n> On 2018-12-11, at 11:28 AM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n> \n> \n> On Tue, Dec 11 2018, Jeff King wrote:\n> \n>> On Mon, Dec 10, 2018 at 07:26:46PM -0800, Stefan Beller wrote:\n>> \n>>>> Context lines do have both. It's just that the default color for context\n>>>> lines is empty. ;)\n>>> \n>>> The content itself can contain color codes.\n>>> \n>>> Instead of unconditionally resetting each line, we could parse each\n>>> content line to determine if we actually have to reset the colors.\n>> \n>> Good point. I don't recall that being the motivation back when this\n>> behavior started, but it's a nice side effect (and the more recent line\n>> you mentioned in emit_line_0 certainly is doing it intentionally).\n>> \n>> That doesn't cover _other_ terminal codes, which could also make for\n>> confusing output, but I do think color codes are somewhat special. We\n>> generally send patches through \"less -R\", which will pass through the\n>> colors but show escaped versions of other codes.\n> \n> I wonder if optimizing this one way or the other matters for some\n> terminals. I.e. if we print out some huge diff of thousands of\n> consecutive \"green\" added lines is it faster/slower on some of them to\n> do one \"begin green\" and \"reset\" at the end, or is one line at a time\n> better, or doesn't it matter at all?\n\n"},{"id":"365110","messageId":"A97DD550-BE35-43BA-A181-708B7D065F3F@gmail.com","threadId":"49974","inReplyTo":"EB1AF739-F97B-4905-9736-2A003722AD9A@gmail.com","subject":"Re: Difficulty with parsing colorized diff output","fromName":"George King","fromEmail":"george.w.king@gmail.com","sentAt":"2018-12-11T18:55:09Z","receivedAt":"2018-12-11T18:55:21Z","isPatch":false,"sender":{"key":"george.w.king@gmail.com","avatar":"https://gravatar.com/avatar/9f94a4a17188bfe407b5d04a8d2563760028f802a0536e61c076dcb806f23226?d=mp&s=160"},"body":"I just noticed that while `wsErrorHighlight = none` fixes the problem of extra green codes for regular diff, it fails to have any effect during interactive `git add -p`.\n\n\n> On 2018-12-11, at 11:41 AM, George King <george.w.king@gmail.com> wrote:\n> \n> I first started playing around with terminal colors about 5 years ago, and I recall learning the hard way that Apple Terminal at least behaves very strangely when you have background colors cross line boundaries: background colors disappeared when I scrolled lines back into view. I filed a bug thinking it couldn't be right and Apple closed it as behaving according to compatibility expectations. I never figured out whether they had misunderstood my report or if old terminals were just that crazy. Instead I decided that the safe thing to do was reset after every line. Perhaps some git author reached the same conclusion.\n> \n> From the perspective of parsing this output, it is really much easier if each line can be understood without considering state of previous lines. If anything, I think it is a safe approach to ensuring that it renders correctly on various terminals as well.\n> \n>> On 2018-12-11, at 11:28 AM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n>> \n>> \n>> On Tue, Dec 11 2018, Jeff King wrote:\n>> \n>>> On Mon, Dec 10, 2018 at 07:26:46PM -0800, Stefan Beller wrote:\n>>> \n>>>>> Context lines do have both. It's just that the default color for context\n>>>>> lines is empty. ;)\n>>>> \n>>>> The content itself can contain color codes.\n>>>> \n>>>> Instead of unconditionally resetting each line, we could parse each\n>>>> content line to determine if we actually have to reset the colors.\n>>> \n>>> Good point. I don't recall that being the motivation back when this\n>>> behavior started, but it's a nice side effect (and the more recent line\n>>> you mentioned in emit_line_0 certainly is doing it intentionally).\n>>> \n>>> That doesn't cover _other_ terminal codes, which could also make for\n>>> confusing output, but I do think color codes are somewhat special. We\n>>> generally send patches through \"less -R\", which will pass through the\n>>> colors but show escaped versions of other codes.\n>> \n>> I wonder if optimizing this one way or the other matters for some\n>> terminals. I.e. if we print out some huge diff of thousands of\n>> consecutive \"green\" added lines is it faster/slower on some of them to\n>> do one \"begin green\" and \"reset\" at the end, or is one line at a time\n>> better, or doesn't it matter at all?\n> \n\n"},{"id":"365170","messageId":"20181212124908.GA5380@sigill.intra.peff.net","threadId":"49974","inReplyTo":"EB1AF739-F97B-4905-9736-2A003722AD9A@gmail.com","subject":"Re: Difficulty with parsing colorized diff output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-12-12T12:49:09Z","receivedAt":"2018-12-12T12:49:12Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Dec 11, 2018 at 11:41:18AM -0500, George King wrote:\n\n> I first started playing around with terminal colors about 5 years ago,\n> and I recall learning the hard way that Apple Terminal at least\n> behaves very strangely when you have background colors cross line\n> boundaries: background colors disappeared when I scrolled lines back\n> into view. I filed a bug thinking it couldn't be right and Apple\n> closed it as behaving according to compatibility expectations. I never\n> figured out whether they had misunderstood my report or if old\n> terminals were just that crazy. Instead I decided that the safe thing\n> to do was reset after every line. Perhaps some git author reached the\n> same conclusion.\n\nYes, that's exactly it. Certain codes do not do well as they cross\nlines. It's been long enough that I don't remember the details, and a\nquick grep of the archive found too many results for me to bother wading\nthrough.\n\n-Peff\n"},{"id":"365174","messageId":"20181212135255.GA5768@sigill.intra.peff.net","threadId":"49974","inReplyTo":"A97DD550-BE35-43BA-A181-708B7D065F3F@gmail.com","subject":"Re: Difficulty with parsing colorized diff output","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-12-12T13:52:55Z","receivedAt":"2018-12-12T13:52:58Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Dec 11, 2018 at 01:55:09PM -0500, George King wrote:\n\n> I just noticed that while `wsErrorHighlight = none` fixes the problem\n> of extra green codes for regular diff, it fails to have any effect\n> during interactive `git add -p`.\n\nThis is due to the way add--interactive invokes diff. It uses the\nplumbing commands (diff-tree, diff-files, etc), which do not respect\nconfig which changes the output (since their purpose is to provide\nstable machine-readable output).\n\nBut add--interactive does a slightly funny thing: it runs each diff\ntwice. Once to get the machine-readable version, and once to get a\ncolorized version which it shows to the user. When it does the latter,\nit has to manually enable other options (e.g., passing --color as\nappropriate, or passing along diff.algorithm).\n\nSo the matching behavior here would be for it to look as\nwsErrorHighlight and pass it along as a command line option.\n\n-Peff\n"}]}