On Mon, Aug 13, 2018 at 4:47 AM Johannes Schindelin
<Johannes.Schindelin@xxxxxx> wrote:
>
> Hi Steafn,
>
> On Fri, 10 Aug 2018, Stefan Beller wrote:
>
> > This will prove useful in range-diff in a later patch as we will be able to
> > differentiate between adding a new file (that line is starting with +++
> > and then the file name) and regular new lines.
>
> Very good!
>
> > It could also be useful for experimentation in new patch formats, i.e.
> > we could teach git to emit moved lines with lines other than +/-.
> >
> > Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
> > ---
> > diff.c | 21 +++++++++++++++++----
> > diff.h | 5 +++++
> > 2 files changed, 22 insertions(+), 4 deletions(-)
> >
> > diff --git a/diff.c b/diff.c
> > index b3cb73eb69a..b75eb085cb3 100644
> > --- a/diff.c
> > +++ b/diff.c
> > @@ -1237,7 +1237,7 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,
> > struct emitted_diff_symbol *eds)
> > {
> > static const char *nneof = " No newline at end of file\n";
> > - const char *context, *reset, *set, *set_sign, *meta, *fraginfo;
> > + const char *context, *reset, *set, *set_sign, *meta, *fraginfo, *first;
> > struct strbuf sb = STRBUF_INIT;
> >
> > enum diff_symbol s = eds->s;
> > @@ -1288,7 +1288,9 @@ static void emit_diff_symbol_from_struct(struct diff_options *o,
> > else if (c == '-')
> > set = diff_get_color_opt(o, DIFF_FILE_OLD);
> > }
> > - emit_line_ws_markup(o, set_sign, set, reset, " ", line, len,
> > + first = o->output_indicators[OI_CONTEXT] ?
> > + o->output_indicators[OI_CONTEXT] : " ";
> > + emit_line_ws_markup(o, set_sign, set, reset, first, line, len,
>
> Instead of doing this over and over again, how about
>
> 1) setting o->output_indicators to " " in diff_setup()?
... and when parsing the command line options we could already overwrite it
in place.
> 2) passing OI_CONTEXT to emit_line_ws_markup() instead of `first`? I.e.
> change it to the index in the output_indicators, with -1 indicating
> "none"?
That sounds like an elegant design, as then it is super clear that 'first'
can only ever be a sign (or character that we chose), but
giving -1 for "none" sounds cumbersome. I'll take a look into that.
> > +#define OI_NEW 0
> > +#define OI_OLD 1
> > +#define OI_CONTEXT 2
>
> I could imagine that OI_* is too generic a prefix, and that we would want
> to have a prefix that is less prone to collide with other global
> constants, such as OUTPUT_INDICATOR_*.
I agree on that; will take the suggestion of
OUTPUT_INDICATOR_*.
