git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 1/3] diff.c: add --output-indicator-{new, old, context}

From
Stefan Beller <sbeller@google.com>
Date
Aug 20, 2018, 19:39 UTC
Message-ID
<CAGZ79kY8oadQ2nKWr7znMa8QJgLcidb0FefPDBaba+q2J=nfPg@mail.gmail.com>
In-Reply-To
<nycvar.QRO.7.76.6.1808202129420.73@tvgsbejvaqbjf.bet>

On Mon, Aug 20, 2018 at 12:31 PM Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 36 quoted lines
>
> Hi Stefan,
>
> On Fri, 17 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.
> >
> > 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 +/-.
>
> Thanks.
>
> > diff --git a/diff.c b/diff.c
> > index c5c7739ce34..03486c35b75 100644
> > --- a/diff.c
> > +++ b/diff.c
> > @@ -1281,7 +1281,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,
>                                     ^
> Here we already pass `o`... so...
>
> > +             emit_line_ws_markup(o, set_sign, set, reset,
> > +                                 o->output_indicators[OUTPUT_INDICATOR_CONTEXT],
>                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> ... here, we could simply pass `OUTPUT_INDICATOR_CONTEXT` and let the
> callee look it up in`o->output_indicators[]`...
>
> I read all three patches and did not see a reason why we could not
> simplify the code that way.
>
> Other than that: great!
Thanks!

I considered it, but was put off by the (small) effort of yet another diff refactoring.

I'll include it in a resend if a resend is needed, otherwise I would suggest to make it a patch on top?

Thanks, Stefan

Previous: Johannes SchindelinNext: Johannes Schindelin
Message 20 of 27 in “Better colors in range-diff!”
  1. 0/4 Better colors in range-diff!Stefan Beller, Aug 10, 2018
  2. 1/4 diff.c: emit_line_0 to take string instead of first signStefan Beller, Aug 10, 2018
  3. Johannes SchindelinAug 13, 2018
  4. Stefan BellerAug 13, 2018
  5. 2/4 diff.c: add --output-indicator-{new, old, context}Stefan Beller, Aug 10, 2018
  6. Johannes SchindelinAug 13, 2018
  7. Stefan BellerAug 13, 2018
  8. 3/4 range-diff: make use of different output indicatorsStefan Beller, Aug 10, 2018
  9. Johannes SchindelinAug 13, 2018
  10. Stefan BellerAug 13, 2018
  11. 4/4 range-diff: indent special lines as contextStefan Beller, Aug 10, 2018
  12. Johannes SchindelinAug 13, 2018
  13. Stefan BellerAug 13, 2018
  14. Johannes SchindelinAug 14, 2018
  15. Stefan BellerAug 14, 2018
  16. Johannes SchindelinAug 16, 2018
  17. 0/3 Better colors in range-diffStefan Beller, Aug 17, 2018
  18. 1/3 diff.c: add --output-indicator-{new, old, context}Stefan Beller, Aug 17, 2018
  19. Johannes SchindelinAug 20, 2018
  20. Stefan BellerAug 20, 2018
  21. Johannes SchindelinAug 21, 2018
  22. diff.c: pass sign_index to emit_line_ws_markupStefan Beller, Aug 22, 2018
  23. Johannes SchindelinAug 23, 2018
  24. 2/3 range-diff: make use of different output indicatorsStefan Beller, Aug 17, 2018
  25. 3/3 range-diff: indent special lines as contextStefan Beller, Aug 17, 2018
  26. Junio C HamanoAug 17, 2018
  27. Stefan BellerAug 17, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.