On Tue, Jun 20, 2017 at 1:13 PM, Jonathan Tan <jonathantanmy@xxxxxxxxxx> wrote:
> I just glanced through this file, because it seems similar to the
> versions I have previously reviewed.
Yes, this has not changed much.
The thing that took so long were patches 1-20 to do.
In these later patches (22) the UX was changed slightly.
The mode is zebra instead of alternating for example
> I'll skip patches 23 onwards in this round of review because (i) I would
> be happy if just patches 1-22 were included in the tree
patch 22 just introduced the zebra mode, which contains all
information necessary, the later "dimming" is purely
UI excitement, no further information added.
In former series, I had documentation at each
patch that added a new mode, now I rolled all of
documentation in patch 25. I will add the basic docs
to that patch in a reroll.
> and (ii) those
> patches might end up changing anyway because of review comments in the
> prior patches.
plain, dimming, docs, and this RFC for machine parsable output,
ok with me.
So I'll focus on the first 22 patches.
>
> On Mon, 19 Jun 2017 19:48:12 -0700
> Stefan Beller <sbeller@xxxxxxxxxx> wrote:
>
>> +/* Find blocks of moved code, delegate actual coloring decision to helper */
>> +static void mark_color_as_moved(struct diff_options *o,
>> + struct hashmap *add_lines,
>> + struct hashmap *del_lines)
>> +{
>
> [snip]
>
>> + if (flipped_block)
>> + l->flags |= DIFF_SYMBOL_MOVED_LINE_ZEBRA;
>
> This should probably be DIFF_SYMBOL_MOVED_LINE_ALT. "Zebra" refers to
> both the stripes, not just the alternate stripe.
ok
