Hi -
On 4/14/19 7:27 AM, Michael Platings wrote:
On Sun, 14 Apr 2019 at 11:24, Junio C Hamano <gitster@xxxxxxxxx> 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 "}".
I'm fine with not zeroing the hash, so long as there's some way to mark it.
We could mark with another *, such that if we mark-ignored and
mark-unblamable you get "**hash". You can't have an unblamable that
isn't from an ignored commit, so a single '*' has only one meaning,
based on your config options.
If that works for you all, I can change this to markIgnoredUnblamables
(instead of 'mask') in the next version.
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.
I'd also keep this set. I think the whole reason for these config
options was that everyone has a different preference, but that
preference rarely changes. I don't want to have to type
--mark-ignored-lines every time I run git blame. If I had to, I'd have
to alias git blame or something.
I think having config options for these sorts of things is fine, since
we know already that for a given user+repo, we want the feature on (or
off). But if I have to remove it, then let me know.
Thanks,
Barret
