threads / bug / 24414

[BUG?] blame: Odd -L 1,+0 behavior

Subject: [BUG?] blame: Odd -L 1,+0 behavior

## tl;dr

4 messages between Jul 16, 2010 and Jul 16, 2010.

replies: 3people: 2as markdown or json

Ævar Arnfjörð Bjarmason· Jul 16, 2010, 15:50 UTC · lore
On Fri, Jul 16, 2010 at 15:35, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> git-pickaxe (later git-blame) gained support for the -L/start/,/end/
> form in 2006 (931233bc66 by Junio C Hamano), but nothing was added to
> test this functionality. Change that by adding more -L tests to
> t8003-blame.sh.

I was also going to patch t8003-blame.sh to test for the functionality introduced in 7bd9641df5, but I'm not sure what it's supposed to do:

ok:
    $ git blame cow
    5b22ca36 (Fourth 2005-04-07 15:16:13 -0700 1) ABC
    5b22ca36 (Fourth 2005-04-07 15:16:13 -0700 2) DEF
    018e3134 (Fifth  2005-04-07 15:17:13 -0700 3) XXXX
    5b22ca36 (Fourth 2005-04-07 15:16:13 -0700 4) GHIJK
ok:
    $ git blame -L 1,2 cow
    5b22ca36 (Fourth 2005-04-07 15:16:13 -0700 1) ABC
    5b22ca36 (Fourth 2005-04-07 15:16:13 -0700 2) DEF
ok, +/- are zero-indexed:
    $ git blame -L 1,+2 cow
    5b22ca36 (Fourth 2005-04-07 15:16:13 -0700 1) ABC
    5b22ca36 (Fourth 2005-04-07 15:16:13 -0700 2) DEF
Shouldn't this either print nothing, er be an error:
    $ git blame -L 1,+0 cow
    5b22ca36 (Fourth 2005-04-07 15:16:13 -0700 1) ABC
    5b22ca36 (Fourth 2005-04-07 15:16:13 -0700 2) DEF
same here, line 0-0 prints the whole file:
    $ git blame -L 0,0 cow
    5b22ca36 (Fourth 2005-04-07 15:16:13 -0700 1) ABC
    5b22ca36 (Fourth 2005-04-07 15:16:13 -0700 2) DEF
    018e3134 (Fifth  2005-04-07 15:17:13 -0700 3) XXXX
    5b22ca36 (Fourth 2005-04-07 15:16:13 -0700 4) GHIJK
And these need a better error message:
    $ git blame -L 0,0a cow
    usage: git blame [options] [rev-opts] [rev] [--] file
Is there any use for +/- 0, or should it always be an error?
Junio C Hamano· Jul 16, 2010, 21:11 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [BUG?] blame: Odd -L 1,+0 behavior

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> ok, +/- are zero-indexed:
>
>     $ git blame -L 1,+2 cow

I don't know what "zero-indexed" means, but +2 means "starting from the line I told you earlier, give me two lines". Likewise -2 means "ending at the line I told you earlier, give me two lines". As a side effect of the internal implementation of this logic -L 5,3 means the same thing as -L 3,5 but that is not an intended nor documented behaviour.

> Shouldn't this either print nothing, er be an error:
(multiple)

The parsing code is lax in the sense that rejecting nonsensical input like "-L 10,-100" and "-L 2,+0" as an error was not considered a primary goal. The only error checking it does is to make sure it does not parse numbers that it cannot use (i.e. start from line 30 in a file that does not have that many lines).

Ævar Arnfjörð Bjarmason· Jul 16, 2010, 21:25 UTC · re: Junio C Hamano · lore

Re: [BUG?] blame: Odd -L 1,+0 behavior

On Fri, Jul 16, 2010 at 21:11, Junio C Hamano <gitster@pobox.com> wrote:
Show 9 quoted lines
> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>
>> ok, +/- are zero-indexed:
>>
>>     $ git blame -L 1,+2 cow
>
> I don't know what "zero-indexed" means, but +2 means "starting from the
> line I told you earlier, give me two lines".  Likewise -2 means "ending at
> the line I told you earlier, give me two lines".

From just using it I parsed 1,+1 as "give me line #1 plus $n-1 lines". At first I thought 1,+1 would print two lines (line 1 *plus* one). But it makes sense when you explain it.

> As a side effect of the internal implementation of this logic -L 5,3
> means the same thing as -L 3,5 but that is not an intended nor
> documented behaviour.
How should L 5,3 and 3,5 work? Should the former give an error?
Show 8 quoted lines
>> Shouldn't this either print nothing, er be an error:
> (multiple)
>
> The parsing code is lax in the sense that rejecting nonsensical input like
> "-L 10,-100" and "-L 2,+0" as an error was not considered a primary goal.
> The only error checking it does is to make sure it does not parse numbers
> that it cannot use (i.e. start from line 30 in a file that does not have
> that many lines).
Do you want a patch to make it less lax?
Junio C Hamano· Jul 16, 2010, 22:33 UTC · re: Ævar Arnfjörð Bjarmason · lore

Re: [BUG?] blame: Odd -L 1,+0 behavior

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 5 quoted lines
>> As a side effect of the internal implementation of this logic -L 5,3
>> means the same thing as -L 3,5 but that is not an intended nor
>> documented behaviour.
>
> How should L 5,3 and 3,5 work? Should the former give an error?

"-L 3,5" would choose "lines 3,4, and 5" (both ends inclusive). Currently "-L 5,3" does the same thing but as I said, that is not an intended nor documented behaviour, and if you are tackling this area to tighten the parsing and error diagnosis, I think it is reasonable to error it out.

Show 10 quoted lines
>>> Shouldn't this either print nothing, er be an error:
>> (multiple)
>>
>> The parsing code is lax in the sense that rejecting nonsensical input like
>> "-L 10,-100" and "-L 2,+0" as an error was not considered a primary goal.
>> The only error checking it does is to make sure it does not parse numbers
>> that it cannot use (i.e. start from line 30 in a file that does not have
>> that many lines).
>
> Do you want a patch to make it less lax?
Be my guest ;-)  Thanks.

← back to recent threads