Output from "git blame A..B -- path" for the bottom commit is misleading

Output from "git blame A..B -- path" for the bottom commit is misleading

[Junio C Hamano]

If you run

    $ git blame -L103,107 v2.0.0-rc0..v2.0.0-rc2 t/t9117-git-svn-init-clone.sh

you will see something like this:

    ^cc29195 (Junio C Hamano 2014-04-18 11:21:43 -0700 103) 
    7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 104) test_expect_...
    ^cc29195 (Junio C Hamano 2014-04-18 11:21:43 -0700 105)         test...
    7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 106)         git ...
    ^cc29195 (Junio C Hamano 2014-04-18 11:21:43 -0700 107)         test...

It is correct to attribute these lines that have not changed since
the bottom of the range (i.e. v2.0.0-rc0) to that commit, and it may
be technically correct to show my name because I recorded the tree
that contains these lines as v2.0.0-rc0 with that commit.

But I find it really misleading, as this is the true picture if we
dug to the bottom of the history:

    $ git blame -L103,107 v2.0.0-rc2 t/t9117-git-svn-init-clone.sh
    f849bb6b (Johan Herland 2013-10-11 14:57:06 +0200 103) 
    7bbc458b (Kyle J. McKay 2014-04-22 04:16:22 -0700 104) test_expect_...
    f849bb6b (Johan Herland 2013-10-11 14:57:06 +0200 105)  test ! -d p...
    7bbc458b (Kyle J. McKay 2014-04-22 04:16:22 -0700 106)  git svn ini...
    f849bb6b (Johan Herland 2013-10-11 14:57:06 +0200 107)  test_must_f...

I do not expect Johan's name to appear in the output for the first
one, because that would require us to dig deeper than the commit we
were told to stop at, but I am wondering if we can do better than
the existing "-b" option to reduce the confusion from the output.

The "-b" option blanks the commit object name, but still shows the
name and timestamp for the bottom commit:

             (Junio C Hamano 2014-04-18 11:21:43 -0700 103) 
    7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 104) test_expect_...
             (Junio C Hamano 2014-04-18 11:21:43 -0700 105)         test...
    7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 106)         git ...
             (Junio C Hamano 2014-04-18 11:21:43 -0700 107)         test...

I am tempted to say "blame that is run without the --porcelain
option is a end-user facing Porcelain, and people should not be
reading its output in their scripts" and change the behaviour of the
"-b" option to instead show something like this instead:
    
    ^cc29195 (Unknown        2014-04-18 11:21:43 -0700 103) 
    7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 104) test_expect_...
    ^cc29195 (Unknown        2014-04-18 11:21:43 -0700 105)         test...
    7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 106)         git ...
    ^cc29195 (Unknown        2014-04-18 11:21:43 -0700 107)         test...

which shows the commit object name, its bottom-ness and the
timestamp, or even

             (                                         103) 
    7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 104) test_expect_...
             (                                         105)         test...
    7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 106)         git ...
             (                                         107)         test...

which does away with the misleading information altogether.

I myself is leaning towards the latter between the two, and not
overriding "-b" but introducing another "cleanse the output of
useless bottom information even more" option.

Opinions?

Re: Output from "git blame A..B -- path" for the bottom commit is misleading

[David Kastrup]

Junio C Hamano <gitster@pobox.com> writes:

> If you run
>
>     $ git blame -L103,107 v2.0.0-rc0..v2.0.0-rc2 t/t9117-git-svn-init-clone.sh
>
> you will see something like this:
>
>     ^cc29195 (Junio C Hamano 2014-04-18 11:21:43 -0700 103) 
>     7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 104) test_expect_...
>     ^cc29195 (Junio C Hamano 2014-04-18 11:21:43 -0700 105)         test...
>     7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 106)         git ...
>     ^cc29195 (Junio C Hamano 2014-04-18 11:21:43 -0700 107)         test...
>
> It is correct to attribute these lines that have not changed since
> the bottom of the range (i.e. v2.0.0-rc0) to that commit,

They are not attributed to that commit in particular.  They are traced
all the way up to that commit, so they are either from this commit or
from an earlier one.

> But I find it really misleading, as this is the true picture if we
> dug to the bottom of the history:
>
>     $ git blame -L103,107 v2.0.0-rc2 t/t9117-git-svn-init-clone.sh
>     f849bb6b (Johan Herland 2013-10-11 14:57:06 +0200 103) 
>     7bbc458b (Kyle J. McKay 2014-04-22 04:16:22 -0700 104) test_expect_...
>     f849bb6b (Johan Herland 2013-10-11 14:57:06 +0200 105)  test ! -d p...
>     7bbc458b (Kyle J. McKay 2014-04-22 04:16:22 -0700 106)  git svn ini...
>     f849bb6b (Johan Herland 2013-10-11 14:57:06 +0200 107)  test_must_f...

So indeed, an earlier one.

> I do not expect Johan's name to appear in the output for the first
> one, because that would require us to dig deeper than the commit we
> were told to stop at, but I am wondering if we can do better than the
> existing "-b" option to reduce the confusion from the output.

Do you mean "better than" or rather "better in the case where -b is
given"?

> The "-b" option blanks the commit object name, but still shows the
> name and timestamp for the bottom commit:
>
>              (Junio C Hamano 2014-04-18 11:21:43 -0700 103) 
>     7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 104) test_expect_...
>              (Junio C Hamano 2014-04-18 11:21:43 -0700 105)         test...
>     7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 106)         git ...
>              (Junio C Hamano 2014-04-18 11:21:43 -0700 107)         test...
>
> I am tempted to say "blame that is run without the --porcelain
> option is a end-user facing Porcelain, and people should not be
> reading its output in their scripts" and change the behaviour of the
> "-b" option to instead show something like this instead:
>     
>     ^cc29195 (Unknown        2014-04-18 11:21:43 -0700 103) 
>     7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 104) test_expect_...
>     ^cc29195 (Unknown        2014-04-18 11:21:43 -0700 105)         test...
>     7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 106)         git ...
>     ^cc29195 (Unknown        2014-04-18 11:21:43 -0700 107)         test...
>
> which shows the commit object name, its bottom-ness and the
> timestamp, or even
>
>              (                                         103) 
>     7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 104) test_expect_...
>              (                                         105)         test...
>     7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 106)         git ...
>              (                                         107)         test...
>
> which does away with the misleading information altogether.

That would make more sense for -b but hardly for the default.

> I myself is leaning towards the latter between the two, and not
> overriding "-b" but introducing another "cleanse the output of
> useless bottom information even more" option.

One could use -b -b but frankly, where is the point?  Name and date
deliver no useful information when the commit is absent.

-- 
David Kastrup

Re: Output from "git blame A..B -- path" for the bottom commit is misleading

[Jeff King]

On Thu, May 08, 2014 at 01:52:38PM -0700, Junio C Hamano wrote:

>              (                                         103) 
>     7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 104) test_expect_...
>              (                                         105)         test...
>     7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 106)         git ...
>              (                                         107)         test...
> 
> which does away with the misleading information altogether.
> 
> I myself is leaning towards the latter between the two, and not
> overriding "-b" but introducing another "cleanse the output of
> useless bottom information even more" option.

Though I rarely use boundary commits, this one makes the most sense to
me (when I do use them, I just mentally assume that the information in
the boundary line is useless; this is just making that more apparent).

Coincidentally, I recently came across a malformed commit that had a
bogus empty committer name and email. The "git log" pretty-printer omits
the author and committer lines entirely. "blame" will show "(unknown)"
in the name field. I wonder if it should also switch to a formatted
blank as above (but _do_ print the commit).

-Peff

Re: Output from "git blame A..B -- path" for the bottom commit is misleading

[David Kastrup]

Jeff King <peff@peff.net> writes:

> On Thu, May 08, 2014 at 01:52:38PM -0700, Junio C Hamano wrote:
>
>>              (                                         103) 
>>     7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 104) test_expect_...
>>              (                                         105)         test...
>>     7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 106)         git ...
>>              (                                         107)         test...
>> 
>> which does away with the misleading information altogether.
>> 
>> I myself is leaning towards the latter between the two, and not
>> overriding "-b" but introducing another "cleanse the output of
>> useless bottom information even more" option.
>
> Though I rarely use boundary commits, this one makes the most sense to
> me (when I do use them, I just mentally assume that the information in
> the boundary line is useless; this is just making that more apparent).

It is unclear to me what "this one makes the most sense to me" is
referring to, in particular whether it encompasses the "and not
overriding" part of the paragraph.

-- 
David Kastrup

Re: Output from "git blame A..B -- path" for the bottom commit is misleading

[John Keeping]

On Thu, May 08, 2014 at 01:52:38PM -0700, Junio C Hamano wrote:
> I am tempted to say "blame that is run without the --porcelain
> option is a end-user facing Porcelain, and people should not be
> reading its output in their scripts" and change the behaviour of the
> "-b" option to instead show something like this instead:
>     
>     ^cc29195 (Unknown        2014-04-18 11:21:43 -0700 103) 
>     7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 104) test_expect_...
>     ^cc29195 (Unknown        2014-04-18 11:21:43 -0700 105)         test...
>     7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 106)         git ...
>     ^cc29195 (Unknown        2014-04-18 11:21:43 -0700 107)         test...
> 
> which shows the commit object name, its bottom-ness and the
> timestamp, or even
> 
>              (                                         103) 
>     7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 104) test_expect_...
>              (                                         105)         test...
>     7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 106)         git ...
>              (                                         107)         test...
> 
> which does away with the misleading information altogether.
> 
> I myself is leaning towards the latter between the two, and not
> overriding "-b" but introducing another "cleanse the output of
> useless bottom information even more" option.

I'd be tempted to leave the SHA-1 to indicate "sometime before" but
delete the author and date (I also think it looks a bit nicer to omit
the opening parenthesis):

    ^cc29195                                           103) 
    7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 104) test_expect_...
    ^cc29195                                           105)         test...
    7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 106)         git ...
    ^cc29195                                           107)         test...


On a slight tangent, I tried this in a fairly young repository and got
this (with master at v2.0.0-rc2-4-g1dc51c6):

$ git blame Makefile | head -5
7a3fc144 (John Keeping      2013-12-26 17:37:53 +0000   1) REL_VERSION = v0.2
5c9829f9 (John Keeping      2013-07-29 17:03:26 +0100   2) 
5c9829f9 (John Keeping      2013-07-29 17:03:26 +0100   3) # The default target is...
^f7fae99 (John Keeping      2013-03-24 17:14:40 +0000   4) all::
^f7fae99 (John Keeping      2013-03-24 17:14:40 +0000   5) 

f7fae99 is the initial commit in the repository, so shouldn't the last
two lines blame to that, not a non-existent ancestor?

Re: Output from "git blame A..B -- path" for the bottom commit is misleading

[Junio C Hamano]

In short:

 - I am not considering nor proposing to change the default at all.

 - I have two choices, either change the behaviour of "-b", or
   introducing a new option (the latter includes "-b -b"); I am
   slightly in favor of the latter, but not by a large margin.

 - I have two choices, regardless of how the new mode is triggered,
   for outputs.  Either fill it with "Unknown" name and leave
   everything else as is, or blank all information from the boundary
   commit out.  I am moderately in favor of the latter.

Hope that clarifies.

Re: Output from "git blame A..B -- path" for the bottom commit is misleading

[Junio C Hamano]

John Keeping <john@keeping.me.uk> writes:

> On a slight tangent, I tried this in a fairly young repository and got
> this (with master at v2.0.0-rc2-4-g1dc51c6):
>
> $ git blame Makefile | head -5
> 7a3fc144 (John Keeping      2013-12-26 17:37:53 +0000   1) REL_VERSION = v0.2
> 5c9829f9 (John Keeping      2013-07-29 17:03:26 +0100   2) 
> 5c9829f9 (John Keeping      2013-07-29 17:03:26 +0100   3) # The default target is...
> ^f7fae99 (John Keeping      2013-03-24 17:14:40 +0000   4) all::
> ^f7fae99 (John Keeping      2013-03-24 17:14:40 +0000   5) 
>
> f7fae99 is the initial commit in the repository, so shouldn't the last
> two lines blame to that, not a non-existent ancestor?

It is not saying f7fae99^, is it?  It is debatable if it is correct
to mark the root commit as a boundary, but that is what it is
showing, I think.  In other words, "this line hasn't changed since
the inception of the project".

Re: Output from "git blame A..B -- path" for the bottom commit is misleading

[John Keeping]

On Thu, May 08, 2014 at 02:58:58PM -0700, Junio C Hamano wrote:
> John Keeping <john@keeping.me.uk> writes:
> 
> > On a slight tangent, I tried this in a fairly young repository and got
> > this (with master at v2.0.0-rc2-4-g1dc51c6):
> >
> > $ git blame Makefile | head -5
> > 7a3fc144 (John Keeping      2013-12-26 17:37:53 +0000   1) REL_VERSION = v0.2
> > 5c9829f9 (John Keeping      2013-07-29 17:03:26 +0100   2) 
> > 5c9829f9 (John Keeping      2013-07-29 17:03:26 +0100   3) # The default target is...
> > ^f7fae99 (John Keeping      2013-03-24 17:14:40 +0000   4) all::
> > ^f7fae99 (John Keeping      2013-03-24 17:14:40 +0000   5) 
> >
> > f7fae99 is the initial commit in the repository, so shouldn't the last
> > two lines blame to that, not a non-existent ancestor?
> 
> It is not saying f7fae99^, is it?  It is debatable if it is correct
> to mark the root commit as a boundary, but that is what it is
> showing, I think.  In other words, "this line hasn't changed since
> the inception of the project".

Yes, it's marking it as a boundary but I'm not convinced that's correct.
Compare these two cases:

$ git blame Makefile | head -5
7a3fc144 (John Keeping      2013-12-26 17:37:53 +0000   1) REL_VERSION = v0.2
5c9829f9 (John Keeping      2013-07-29 17:03:26 +0100   2) 
5c9829f9 (John Keeping      2013-07-29 17:03:26 +0100   3) # The default target is...
^f7fae99 (John Keeping      2013-03-24 17:14:40 +0000   4) all::
^f7fae99 (John Keeping      2013-03-24 17:14:40 +0000   5) 

$ git blame ^5c9829f9 Makefile | head -5
7a3fc144 (John Keeping      2013-12-26 17:37:53 +0000   1) REL_VERSION = v0.2
^5c9829f (John Keeping      2013-07-29 17:03:26 +0100   2) 
^5c9829f (John Keeping      2013-07-29 17:03:26 +0100   3) # The default target is...
^5c9829f (John Keeping      2013-07-29 17:03:26 +0100   4) all::
^5c9829f (John Keeping      2013-07-29 17:03:26 +0100   5) 


While it might be useful to mark the initial commit, the current output
seems to mean that lines 4 and 5 existed before the repository was
created.  If you consider blame's output to mean "introduced by commit"
then those lines should simply blame to the initial commit.

`git log --boundary` does not mark the initial commit as a boundary.

Re: Output from "git blame A..B -- path" for the bottom commit is misleading

[John Keeping]

On Thu, May 08, 2014 at 11:10:24PM +0100, John Keeping wrote:
> On Thu, May 08, 2014 at 02:58:58PM -0700, Junio C Hamano wrote:
> > John Keeping <john@keeping.me.uk> writes:
> > 
> > > On a slight tangent, I tried this in a fairly young repository and got
> > > this (with master at v2.0.0-rc2-4-g1dc51c6):
> > >
> > > $ git blame Makefile | head -5
> > > 7a3fc144 (John Keeping      2013-12-26 17:37:53 +0000   1) REL_VERSION = v0.2
> > > 5c9829f9 (John Keeping      2013-07-29 17:03:26 +0100   2) 
> > > 5c9829f9 (John Keeping      2013-07-29 17:03:26 +0100   3) # The default target is...
> > > ^f7fae99 (John Keeping      2013-03-24 17:14:40 +0000   4) all::
> > > ^f7fae99 (John Keeping      2013-03-24 17:14:40 +0000   5) 
> > >
> > > f7fae99 is the initial commit in the repository, so shouldn't the last
> > > two lines blame to that, not a non-existent ancestor?
> > 
> > It is not saying f7fae99^, is it?  It is debatable if it is correct
> > to mark the root commit as a boundary, but that is what it is
> > showing, I think.  In other words, "this line hasn't changed since
> > the inception of the project".
> 
> Yes, it's marking it as a boundary but I'm not convinced that's correct.

But it is intentional.  I get the output I expect if I use
`git blame --root`.

Re: Output from "git blame A..B -- path" for the bottom commit is misleading

[Junio C Hamano]

John Keeping <john@keeping.me.uk> writes:

> Yes, it's marking it as a boundary but I'm not convinced that's correct.
> Compare these two cases:
>
> $ git blame Makefile | head -5
> 7a3fc144 (John Keeping      2013-12-26 17:37:53 +0000   1) REL_VERSION = v0.2
> 5c9829f9 (John Keeping      2013-07-29 17:03:26 +0100   2) 
> 5c9829f9 (John Keeping      2013-07-29 17:03:26 +0100   3) # The default target is...
> ^f7fae99 (John Keeping      2013-03-24 17:14:40 +0000   4) all::
> ^f7fae99 (John Keeping      2013-03-24 17:14:40 +0000   5) 
>
> $ git blame ^5c9829f9 Makefile | head -5
> 7a3fc144 (John Keeping      2013-12-26 17:37:53 +0000   1) REL_VERSION = v0.2
> ^5c9829f (John Keeping      2013-07-29 17:03:26 +0100   2) 
> ^5c9829f (John Keeping      2013-07-29 17:03:26 +0100   3) # The default target is...
> ^5c9829f (John Keeping      2013-07-29 17:03:26 +0100   4) all::
> ^5c9829f (John Keeping      2013-07-29 17:03:26 +0100   5) 
>
>
> While it might be useful to mark the initial commit, the current output
> seems to mean that lines 4 and 5 existed before the repository was
> created.  If you consider blame's output to mean "introduced by commit"
> then those lines should simply blame to the initial commit.

You may be onto something, but I am not sure.  Let me, as you told
me to, compare these two outputs.

The first says "At f7fae99, these lines were already there, and we
are not showing anything before that state."  The second says
exactly the same thing but with f7fae99 replaced with 5c9829f.

It is debatable if a root commit is *that* special to be shown
differently from an in-between commit that introduced a change, and
for consistency, I tend to agree with you that it may be better if
roots weren't marked with the "^" prefix.

Re: Output from "git blame A..B -- path" for the bottom commit is misleading

[Jeff King]

On Thu, May 08, 2014 at 11:32:01PM +0200, David Kastrup wrote:

> Jeff King <peff@peff.net> writes:
> 
> > On Thu, May 08, 2014 at 01:52:38PM -0700, Junio C Hamano wrote:
> >
> >>              (                                         103) 
> >>     7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 104) test_expect_...
> >>              (                                         105)         test...
> >>     7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 106)         git ...
> >>              (                                         107)         test...
> >> 
> >> which does away with the misleading information altogether.
> >> 
> >> I myself is leaning towards the latter between the two, and not
> >> overriding "-b" but introducing another "cleanse the output of
> >> useless bottom information even more" option.
> >
> > Though I rarely use boundary commits, this one makes the most sense to
> > me (when I do use them, I just mentally assume that the information in
> > the boundary line is useless; this is just making that more apparent).
> 
> It is unclear to me what "this one makes the most sense to me" is
> referring to, in particular whether it encompasses the "and not
> overriding" part of the paragraph.

Sorry, I should have been more clear. I meant that the output format
shown there makes the most sense of the ones shown.

I'd actually be inclined to say the opposite of what Junio is saying
there: that "-b" should blank the author field as well as the commit
sha1. I'd even go so far as to say that "-b" should probably be the
default when boundary commits are in use. I cannot think of a time when
I have found the boundary information useful, and the IMHO the output
above is less confusing than what we produce now. But I admit I haven't
thought very hard on it.

-Peff

Re: Output from "git blame A..B -- path" for the bottom commit is misleading

[Jeff King]

On Thu, May 08, 2014 at 02:56:46PM -0700, Junio C Hamano wrote:

> In short:
> 
>  - I am not considering nor proposing to change the default at all.
> 
>  - I have two choices, either change the behaviour of "-b", or
>    introducing a new option (the latter includes "-b -b"); I am
>    slightly in favor of the latter, but not by a large margin.
> 
>  - I have two choices, regardless of how the new mode is triggered,
>    for outputs.  Either fill it with "Unknown" name and leave
>    everything else as is, or blank all information from the boundary
>    commit out.  I am moderately in favor of the latter.
> 
> Hope that clarifies.

Thanks, that makes it much more clear to me at least what you were
asking in the original.

Not that my opinion necessarily matters, but what I was trying to
express elsewhere in the thread was a preference for: output is blank,
and tie it to "-b" (mostly because I do not see any way in which the
author name sans commit is at all useful).

I _would_ consider changing the default to "-b", but that is really a
separate topic from this (and quite possibly falls under the "if I were
designing from scratch today, but I am not..." category of changes).

-Peff

Re: Output from "git blame A..B -- path" for the bottom commit is misleading

[David Kastrup]

Jeff King <peff@peff.net> writes:

> I'd actually be inclined to say the opposite of what Junio is saying
> there: that "-b" should blank the author field as well as the commit
> sha1. I'd even go so far as to say that "-b" should probably be the
> default when boundary commits are in use. I cannot think of a time when
> I have found the boundary information useful, and the IMHO the output
> above is less confusing than what we produce now. But I admit I haven't
> thought very hard on it.

Arguably if the user explicitly limited the range, he knows what he's
looking at.  Admittedly, I don't know offhand which options _will_
produce boundary commit indications: there may be some without explicit
range limitation, and we might also be talking about limiting through
shallow repos (git blame on a shallow repo is probably a bad idea in the
first place, but anyway).

-- 
David Kastrup

Re: Output from "git blame A..B -- path" for the bottom commit is misleading

[Jeff King]

On Fri, May 09, 2014 at 07:04:05AM +0200, David Kastrup wrote:

> Arguably if the user explicitly limited the range, he knows what he's
> looking at. Admittedly, I don't know offhand which options _will_
> produce boundary commit indications: there may be some without explicit
> range limitation, and we might also be talking about limiting through
> shallow repos (git blame on a shallow repo is probably a bad idea in the
> first place, but anyway).

Yes, I was thinking mostly of "X..Y" types of ranges, which are probably
the most common. I hadn't considered shallow repositories, and you can
also hit the root commit as a boundary if you do not specify --root.

I guess the question still in my mind is: what use does the identity of
the boundary commit have? That is, whether you know ahead of time where
the boundary is or not, is there ever a case where knowing its author
and/or commit sha1 is a useful piece of information, as opposed to
knowing that we hit a boundary at all?

I could not think of one, but I may simply lack imagination.

-Peff

Re: Output from "git blame A..B -- path" for the bottom commit is misleading

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Fri, May 09, 2014 at 07:04:05AM +0200, David Kastrup wrote:
>
>> Arguably if the user explicitly limited the range, he knows what he's
>> looking at. Admittedly, I don't know offhand which options _will_
>> produce boundary commit indications: there may be some without explicit
>> range limitation, and we might also be talking about limiting through
>> shallow repos (git blame on a shallow repo is probably a bad idea in the
>> first place, but anyway).
>
> Yes, I was thinking mostly of "X..Y" types of ranges, which are probably
> the most common. I hadn't considered shallow repositories, and you can
> also hit the root commit as a boundary if you do not specify --root.
>
> I guess the question still in my mind is: what use does the identity of
> the boundary commit have? That is, whether you know ahead of time where
> the boundary is or not, is there ever a case where knowing its author
> and/or commit sha1 is a useful piece of information, as opposed to
> knowing that we hit a boundary at all?
>
> I could not think of one, but I may simply lack imagination.

Well, the original message was triggered by the same "I could not
think of one" from me ;-).

We may want to flip the default to do a more sanitised version of "-b"
that has been suggested earlier:

>              (                                         103) 
>     7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 104) test_expect_...
>              (                                         105)         test...
>     7bbc458b (Kyle J. McKay  2014-04-22 04:16:22 -0700 106)         git ...
>              (                                         107)         test...
> 
> which does away with the misleading information altogether.

and have another option to show the current default output for those
who would want that information.

But that will be a topic for post 2.0; I should start preparing for
the -rc3 soonish, so I'll stop here.

Re: Output from "git blame A..B -- path" for the bottom commit is misleading

[David Kastrup]

Junio C Hamano <gitster@pobox.com> writes:

> Jeff King <peff@peff.net> writes:
>
>> On Fri, May 09, 2014 at 07:04:05AM +0200, David Kastrup wrote:
>>
>>> Arguably if the user explicitly limited the range, he knows what he's
>>> looking at. Admittedly, I don't know offhand which options _will_
>>> produce boundary commit indications: there may be some without explicit
>>> range limitation, and we might also be talking about limiting through
>>> shallow repos (git blame on a shallow repo is probably a bad idea in the
>>> first place, but anyway).
>>
>> Yes, I was thinking mostly of "X..Y" types of ranges, which are probably
>> the most common. I hadn't considered shallow repositories, and you can
>> also hit the root commit as a boundary if you do not specify --root.
>>
>> I guess the question still in my mind is: what use does the identity of
>> the boundary commit have? That is, whether you know ahead of time where
>> the boundary is or not, is there ever a case where knowing its author
>> and/or commit sha1 is a useful piece of information, as opposed to
>> knowing that we hit a boundary at all?
>>
>> I could not think of one, but I may simply lack imagination.
>
> Well, the original message was triggered by the same "I could not
> think of one" from me ;-).

If it's the root commit, omitting all info may surprisingly make "who
should I yell at" hard.  I also am not sure about the implications in
connection with --reverse.

In connection with explicit -b however, I think it is nonsensical to
blank out only the commit id.

-- 
David Kastrup

Re: Output from "git blame A..B -- path" for the bottom commit is misleading

[Duy Nguyen]

On Fri, May 9, 2014 at 12:04 PM, David Kastrup <dak@gnu.org> wrote:
> Jeff King <peff@peff.net> writes:
>
>> I'd actually be inclined to say the opposite of what Junio is saying
>> there: that "-b" should blank the author field as well as the commit
>> sha1. I'd even go so far as to say that "-b" should probably be the
>> default when boundary commits are in use. I cannot think of a time when
>> I have found the boundary information useful, and the IMHO the output
>> above is less confusing than what we produce now. But I admit I haven't
>> thought very hard on it.
>
> Arguably if the user explicitly limited the range, he knows what he's
> looking at.  Admittedly, I don't know offhand which options _will_
> produce boundary commit indications: there may be some without explicit
> range limitation, and we might also be talking about limiting through
> shallow repos (git blame on a shallow repo is probably a bad idea in the
> first place, but anyway).

No it's not. The idea of shallow repos is to work like a normal repo
(most of the time at least). Excluding git-blame from shallow repos is
a bad idea. Luckily it's not hard to detect shallow boundaries: if a
commit has no parents and lookup_commit_graft() returns -1, then
that's it.
-- 
Duy