> > - fputs(diff_line_prefix(o), file); > > + if (first) > > + fputs(diff_line_prefix(o), file); > > + else if (!len) > > + return; > > Can you explain this hunk in the log message? I am not sure how the > description in the log message relates to this change. Is the idea > of this change essentially "all the existing callers that aren't > doing the diff-of-diffs send a non-NUL first character, and for them > this change is a no-op. New callers share most of the remainder of > emit_line_0() logic but do not want to show the prefix, so the > support for it is piggy-backing by a special case where first could > be NUL"?
All but two caller have 'reverse' set to 0, using the arguments as before. The other two callers are using the function twice to get the prefix and set sign going, and then the second call to get the rest of the line going (which needs to omit the prefix as that was done in the first call) : + /* Emit just the prefix, then the rest. */ + emit_line_0(o, set_sign ? set_sign : set, !!set_sign, reset, + sign, "", 0); + emit_line_0(o, set, 0, reset, 0, line, len); I attempted to clean it up on top, but likely got it wrong as we have no tests for colored range diffs, yet. https://public-inbox.org/git/20180710174552.30123-3-sbel...@google.com/ My suggestion would be to first clarify emit_line_0 and have its arguments and its execution map better to each other, (and as a result only needing to have one call of emit_line_0 instead of two) That is my understanding of the situation. Thanks, Stefan
