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

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

From
BRBarret Rhoden <brho@google.com>
Date
Apr 15, 2019, 13:51 UTC
Message-ID
<1a1b3cd1-5f00-37c9-7382-72de000dd925@google.com>
In-Reply-To
<CAJDYR9TRk99Kwq5S7udVqYsXnupGD=t3o_Ss8ewvwWuTQOy_YQ@mail.gmail.com>
Hi -
On 4/14/19 7:27 AM, Michael Platings wrote:
Show 44 quoted lines
> 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 "}".
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.

Show 10 quoted lines
>> 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
Previous: Michael PlatingsNext: Barret Rhoden
Message 16 of 25 in “blame: add the ability to ignore commits”
  1. 0/6 blame: add the ability to ignore commitsBarret Rhoden, Apr 10, 2019
  2. 1/6 Move init_skiplist() outside of fsckBarret Rhoden, Apr 10, 2019
  3. Ævar Arnfjörð BjarmasonApr 10, 2019
  4. Barret RhodenApr 15, 2019
  5. 2/6 blame: use a helper function in blame_chunk()Barret Rhoden, Apr 10, 2019
  6. 3/6 blame: add the ability to ignore commits and their changesBarret Rhoden, Apr 10, 2019
  7. Ævar Arnfjörð BjarmasonApr 10, 2019
  8. Michael PlatingsApr 14, 2019
  9. Barret RhodenApr 15, 2019
  10. Barret RhodenApr 15, 2019
  11. 4/6 blame: add config options to handle output for ignored linesBarret Rhoden, Apr 10, 2019
  12. Junio C HamanoApr 14, 2019
  13. Michael PlatingsApr 14, 2019
  14. Junio C HamanoApr 14, 2019
  15. Michael PlatingsApr 14, 2019
  16. Barret RhodenApr 15, 2019
  17. 5/6 blame: optionally track line fingerprints during fill_blame_origin()Barret Rhoden, Apr 10, 2019
  18. 6/6 blame: use a fingerprint heuristic to match ignored linesBarret Rhoden, Apr 10, 2019
  19. Junio C HamanoApr 14, 2019
  20. Michael PlatingsApr 14, 2019
  21. Barret RhodenApr 15, 2019
  22. Junio C HamanoApr 16, 2019
  23. Michael PlatingsApr 14, 2019
  24. Barret RhodenApr 15, 2019
  25. Michael PlatingsApr 15, 2019

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.