Re: [RFC/PATCH] builtin/blame: darken redundant line information

From: Stefan Beller <sbeller@google.com>
Date: Jun 13, 2017, 16:21 UTC
Message-ID: <CAGZ79kYR+qh1X-dQixdpDbcr5z-DJ2mkdncaVn_8y90kNco9tw@mail.gmail.com>
In-Reply-To: <xmqqvanz9afq.fsf@gitster.mtv.corp.google.com>

On Tue, Jun 13, 2017 at 8:25 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 14 quoted lines

> Stefan Beller <sbeller@google.com> writes:
>
>> When using git-blame lots of lines contain redundant information, for
>> example in hunks that consist of multiple lines, the metadata (commit name,
>> author, timezone) are repeated. A reader may not be interested in those,
>> so darken them. The darkening is not just based on hunk, but actually
>> takes the previous lines content for that field to compare to.
>>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>
> Not about "blame", but I was trying the --color-moved stuff on
> Brandon's "create config.h" patch and found its behaviour somewhat
> different from what I recall we discussed.

We discussed several things and we did not come to an agreement, what is best, so I implemented different things there.

> I thought that the
> adjacentbounds mode was invented to dim (i.e. not attract undue
> attention) to most of the moved lines, but highlight only the
> boundary of moved blocks,

I agree up to this part. And when you use the standard color scheme, the new/old moved is dimmed according to my perception of colors.

If you use an individual setup for colors, you need to adjust the four color.diff.{old, new}Moved[Alternative] slots.

> so I expected most of the new and old
> lines in that patch would be shown in the "context" color,

So instead of a special color in this mode you expected "context" for color.diff.{old, new}Moved and "highlighted" for the alternative slots.

Show 7 quoted lines

> except
> for the boundary between two blocks of removed lines that have gone
> to different location (and similarly two blocks of new lines that
> have come from different location) would be painted in oldmoved and
> newmoved colors and their alternatives.  Instead I see all old and
> new lines that are moved painted in these colors, without any
> dimming.

http://i.imgur.com/36DMRBl.png is what I see (regular colors, "git show --color-moved f2f1da5348ff2f297b43b") and that is what I would expect.

>
> Is my expectation off?

Maybe? It sounds as if you expected 'context' color to be used in the adjacentbounds.

* Do you expect 'context' to be used in other modes as well?

* Do you expect this as code/algorithm change or would rather
  a change of default colors (default {old/new}Moved = 'context')
  be sufficient to meet that expectation?

* As you have an individual color setup, maybe you can fix this
  for you by setting the appropriate slots to your perception of
  dimmed?

Thanks, Stefan