Re: [PATCH v6 4/6] blame: add config options to handle output for ignored lines

[Michael Platings <michael@platin.gs>]

On Sun, 14 Apr 2019 at 11:24, Junio C Hamano <gitster@pobox.com> wrote:
> > If you only enable blame.markIgnoredLines then the hash for
> > "unblamable" lines appears as e.g. "*3252488f5" - this doesn't seem
> > right to me because the commit *wasn't* ignored,
>
> I think you misunderstood me.  I was merely suggesting to use the
> approach to mark the line in a way other than using the NULLed out
> object name that has been reserved for something totally different,
> and hinting with "the same *idea*".

Hi Junio, that paragraph wasn't targetted at yourself, more a comment
on the functionality as it exists in the latest patch series. Sorry
for not making that clear.

> the "^" marker
> that is used to say "the line is attributed to this commit, but that
> may only be because you blamed with commit range A..B and we reached
> the bottom of the range---if you dug further, you might find the
> line originates from another commit" is the origin of the same idea,
> and this topic borrows it and uses a different mark, i.e. '*', for
> the "we are not certain---take this with grain of salt" mark.

So it sounds like we have many types of blame to consider:

1) This commit is truly the last one to touch this line, and you
didn't ask to ignore it.
2) This commit is truly the last one to touch this line, but you asked
to ignore it (AKA "unblamable").
3) This commit is at the bottom of the range of commits (^)
4) The "true" commit was ignored but we guess this is the one you're
actually interested in (*)
5) The "true" commit was ignored and we've reached the bottom of the
range of commits (^*)?
6) This commit is at the bottom of the range of commits, and you asked
to ignore it.

> If you ended up hitting the commit the user wanted to ignore,
> perhaps you can find another character that is different from '^' or
> '*' and use that, following the same idea.

I personally don't find the "unblamable" lines interesting enough to
justify giving them a symbol. But if Barret strongly feels that such
lines should get a '*' then I won't fight it - these lines tend to be
as simple as "}".

> By the way, a configuration only feature is something we usually do
> not accept.  A feature must be guarded with --command-line-option
> and then optionally can have a corresponding configuration once the
> option proves to be useful enough that it becomes useful to be able
> to say "in this repository (or to this user), the feature is on by
> default".

In that case we definitely need a --mark-ignored-lines option to git
blame, and I would strongly prefer that we also keep the
blame.markIgnoredLines option as I for one will be switching it on.