You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <da...@apache.org> on 2013/06/10 15:47:35 UTC

Re: svn commit: r1491445 - /subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c

On Mon, Jun 10, 2013 at 01:38:48PM -0000, danielsh@apache.org wrote:
> Author: danielsh
> Date: Mon Jun 10 13:38:48 2013
> New Revision: 1491445
> 
> URL: http://svn.apache.org/r1491445
> Log:
> * subversion/libsvn_fs_fs/fs_fs.c: Restore linebreak that was removed, I suspect by accident.
> 

It would have been easy to find what revision removed the line break if we had
a reverse blame --- that is, a blame that walks the chain of diffs from newer
to older, rather than from older to newer.

While we're talking about blame improvements, another one is blame a line
range: stop as soon as every line in a given [X, Y] range is accounted for
(use-case: svn blame | grep -5 '/line I am looking at/').  Bert says that stop
as soon as "at least one" line in a given range would be useful for him
(use-case: "which revision last changed [this function definition]?") and
suggests that API users would find a callback that allows them to decide when
to stop gathering further blame information.



> Modified:
>     subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
> 
> Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c?rev=1491445&r1=1491444&r2=1491445&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Mon Jun 10 13:38:48 2013
> @@ -484,6 +484,7 @@ path_and_offset_of(apr_file_t *file, apr
>  
>  
>  
> +
>  /* Functions for working with shared transaction data. */
>  
>  /* Return the transaction object for transaction TXN_ID from the
> 
> 

Re: Blame improvements - line range; deleted lines [was: svn commit: r1491445 ...]

Posted by Johan Corveleyn <jc...@gmail.com>.
On Wed, Jun 12, 2013 at 2:25 PM, Julian Foad <ju...@btopenworld.com> wrote:
> Markus Schaber wrote:
>
>> On Mon, Jun 10, 2013 at 01:38:48PM -0000, danielsh@apache.org wrote:
>>> It would have been easy to find what revision removed the line break if we
>>> had a reverse blame --- that is, a blame that walks the chain of diffs from
>>> newerto older, rather than from older to newer.
>>
>> +1
>>
>>> While we're talking about blame improvements, another one is blame a line
>>> range: stop as soon as every line in a given [X, Y] range is accounted for
>>> (use-case: svn blame | grep -5 '/line I am looking at/').  Bert says that
>>> stop  as soon as "at least one" line in a given range would be useful for
>>> him (use-case: "which revision last changed [this function definition]?")
>>> and suggests that API users would find a callback that allows them to decide
>>> when to stop gathering further blame information.
>>
>> Agreed.
>>
>> May be this way also allows to do queries for deleted lines by making the
>> callback stop when he finds a deleted line in a given range.
>
> I have thought before that it would sometimes be useful to include blame information on the gaps between lines.  For each gap between adjacent lines (and before the first and after the last line), there is a revision in which any text between these two lines was deleted.
>
> Example: blame -r1:30 foo.c@30 could produce these (revnum | line-text) pairs:
>   r20 | "int main()"
>   r20 | "{"
>   r25 | NULL
>   r30 | "    return 0;"
>   r20 | "}"
>
> where NULL means some line(s) were at this position in r24 but nothing was here in r25 up to the revision being blamed.
>

I'm not sure about your idea, Julian (still have to wrap my head
around it), but I'll try to give some more context about Daniel's
suggestion:

Just to clarify: there are now two notions of "reverse blame".

1) Give me normal blame output (i.e. "show me the lines of the working
file (or the youngest file in the given revision range), and show me
at each line the revnumber where it was last added"), *but* do it
walking the history backwards. So the output should be exactly the
same as "normal blame" (no visible effect for the user), but we
optimize the algorithm, and (possibly) have the ability to give
partial information to the user early (e.g. a GUI could gradually show
the revnumbers next to the lines, as they come in, calculated by the
"backwards walking" algorithm). Also, we'd have the ability to stop
the calculation early, if all the lines have been "blamed".

2) Give me "reverse blame" output, meaning: show me lines of the
oldest file in the given revision range, and show me at each line the
revnumber where the line was last present. That allows you to see
where a line was deleted. So in this case the user is actually asking
the reverse question (it's like you change the direction of the time
arrow, and then request a "normal blame").

Perhaps 1 should grow another name, like "backward walking blame
algorithm" or .... It's really an implementation detail, most users
will be unaware of it. It'll be important for API users though (at
least if we want to expose the partial information stuff).

The two notions are perfectly orthogonal. The "algorithm reversion"
can also be applied to the "reverse blame".

--
Johan

Re: Blame improvements - line range; deleted lines [was: svn commit: r1491445 ...]

Posted by Ben Reser <be...@reser.org>.
On Wed, Jun 12, 2013 at 5:52 PM, Daniel Shahaf <da...@elego.de> wrote:
> What about an optional output mode that prints the gaps only, but all of
> them?
>
> e.g., if I do 'blame -r 25:29', the optional output mode would print all
> lines added in r26 or in r27 or in r28 that are not present in r29, and
> only them.

Isn't that what diff does?

Re: Blame improvements - line range; deleted lines [was: svn commit: r1491445 ...]

Posted by Daniel Shahaf <da...@elego.de>.
Julian Foad wrote on Wed, Jun 12, 2013 at 16:41:59 +0100:
> Prabhu wrote:
> 
> > On 06/12/2013 05:55 PM, Julian Foad wrote:
> >>  I have thought before that it would sometimes be useful to include blame 
> >> information on the gaps between lines.  For each gap between adjacent lines (and 
> >> before the first and after the last line), there is a revision in which any text 
> >> between these two lines was deleted.
> >> 
> >>  Example: blame -r1:30 foo.c@30 could produce these (revnum | line-text) 
> >> pairs:
> >>     r20 | "int main()"
> >>     r20 | "{"
> >>     r25 | NULL
> >>     r30 | "    return 0;"
> >>     r20 | "}"
> >> 
> >>  where NULL means some line(s) were at this position in r24 but nothing was 
> >> here in r25 up to the revision being blamed.
> > 
> > How would we handle/show if the line existed in 24r but not in r25.. 
> > Again existed in r27 and removed in r29 ? I can't think of the UI...
> 
> Blame is only intended to identify the most recent change to each line.  In this case, we would identify the most recent change of the inter-line gap, so that would be r29.  I think that is useful and consistent with the way Blame works already.
> 
> As a different example, if there were two lines in that "gap" in r24, and one of them was removed in r25, and the second one was removed in r29, then again we'd only show "r29" for the gap, because that is the most recent change in that gap.

What about an optional output mode that prints the gaps only, but all of
them?

e.g., if I do 'blame -r 25:29', the optional output mode would print all
lines added in r26 or in r27 or in r28 that are not present in r29, and
only them.

Re: Blame improvements - line range; deleted lines [was: svn commit: r1491445 ...]

Posted by Julian Foad <ju...@btopenworld.com>.
Prabhu wrote:

> On 06/12/2013 05:55 PM, Julian Foad wrote:
>>  I have thought before that it would sometimes be useful to include blame 
>> information on the gaps between lines.  For each gap between adjacent lines (and 
>> before the first and after the last line), there is a revision in which any text 
>> between these two lines was deleted.
>> 
>>  Example: blame -r1:30 foo.c@30 could produce these (revnum | line-text) 
>> pairs:
>>     r20 | "int main()"
>>     r20 | "{"
>>     r25 | NULL
>>     r30 | "    return 0;"
>>     r20 | "}"
>> 
>>  where NULL means some line(s) were at this position in r24 but nothing was 
>> here in r25 up to the revision being blamed.
> 
> How would we handle/show if the line existed in 24r but not in r25.. 
> Again existed in r27 and removed in r29 ? I can't think of the UI...

Blame is only intended to identify the most recent change to each line.  In this case, we would identify the most recent change of the inter-line gap, so that would be r29.  I think that is useful and consistent with the way Blame works already.

As a different example, if there were two lines in that "gap" in r24, and one of them was removed in r25, and the second one was removed in r29, then again we'd only show "r29" for the gap, because that is the most recent change in that gap.

- Julian

Re: Blame improvements - line range; deleted lines [was: svn commit: r1491445 ...]

Posted by Prabhu <pr...@collab.net>.
On 06/12/2013 05:55 PM, Julian Foad wrote:
> Markus Schaber wrote:
>
>> On Mon, Jun 10, 2013 at 01:38:48PM -0000, danielsh@apache.org wrote:
>>> It would have been easy to find what revision removed the line break if we
>>> had a reverse blame --- that is, a blame that walks the chain of diffs from
>>> newerto older, rather than from older to newer.
>> +1
>>
>>> While we're talking about blame improvements, another one is blame a line
>>> range: stop as soon as every line in a given [X, Y] range is accounted for
>>> (use-case: svn blame | grep -5 '/line I am looking at/').  Bert says that
>>> stop  as soon as "at least one" line in a given range would be useful for
>>> him (use-case: "which revision last changed [this function definition]?")
>>> and suggests that API users would find a callback that allows them to decide
>>> when to stop gathering further blame information.
>> Agreed.
>>
>> May be this way also allows to do queries for deleted lines by making the
>> callback stop when he finds a deleted line in a given range.
> I have thought before that it would sometimes be useful to include blame information on the gaps between lines.  For each gap between adjacent lines (and before the first and after the last line), there is a revision in which any text between these two lines was deleted.
>
> Example: blame -r1:30 foo.c@30 could produce these (revnum | line-text) pairs:
>    r20 | "int main()"
>    r20 | "{"
>    r25 | NULL
>    r30 | "    return 0;"
>    r20 | "}"
>
> where NULL means some line(s) were at this position in r24 but nothing was here in r25 up to the revision being blamed.
>
> - Julian


How would we handle/show if the line existed in 24r but not in r25.. 
Again existed in r27 and removed in r29 ? I can't think of the UI...


--Prabhu

Blame improvements - line range; deleted lines [was: svn commit: r1491445 ...]

Posted by Julian Foad <ju...@btopenworld.com>.
Markus Schaber wrote:

> On Mon, Jun 10, 2013 at 01:38:48PM -0000, danielsh@apache.org wrote:
>> It would have been easy to find what revision removed the line break if we 
>> had a reverse blame --- that is, a blame that walks the chain of diffs from 
>> newerto older, rather than from older to newer.
> 
> +1
> 
>> While we're talking about blame improvements, another one is blame a line
>> range: stop as soon as every line in a given [X, Y] range is accounted for
>> (use-case: svn blame | grep -5 '/line I am looking at/').  Bert says that
>> stop  as soon as "at least one" line in a given range would be useful for
>> him (use-case: "which revision last changed [this function definition]?")
>> and suggests that API users would find a callback that allows them to decide 
>> when to stop gathering further blame information.
> 
> Agreed.
> 
> May be this way also allows to do queries for deleted lines by making the 
> callback stop when he finds a deleted line in a given range.

I have thought before that it would sometimes be useful to include blame information on the gaps between lines.  For each gap between adjacent lines (and before the first and after the last line), there is a revision in which any text between these two lines was deleted.

Example: blame -r1:30 foo.c@30 could produce these (revnum | line-text) pairs:
  r20 | "int main()"
  r20 | "{"
  r25 | NULL
  r30 | "    return 0;"
  r20 | "}"

where NULL means some line(s) were at this position in r24 but nothing was here in r25 up to the revision being blamed.

- Julian

AW: svn commit: r1491445 - /subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c

Posted by Markus Schaber <m....@codesys.com>.
Hi,

On Mon, Jun 10, 2013 at 01:38:48PM -0000, danielsh@apache.org wrote:
>> Author: danielsh
>> Date: Mon Jun 10 13:38:48 2013
>> New Revision: 1491445
>>
>> URL: http://svn.apache.org/r1491445
>> Log:
>> * subversion/libsvn_fs_fs/fs_fs.c: Restore linebreak that was removed, I suspect by accident.
>>

> It would have been easy to find what revision removed the line break if we had
> a reverse blame --- that is, a blame that walks the chain of diffs from newer
> to older, rather than from older to newer.

+1

> While we're talking about blame improvements, another one is blame a line
> range: stop as soon as every line in a given [X, Y] range is accounted for
> (use-case: svn blame | grep -5 '/line I am looking at/').  Bert says that stop
> as soon as "at least one" line in a given range would be useful for him
> (use-case: "which revision last changed [this function definition]?") and
> suggests that API users would find a callback that allows them to decide when
> to stop gathering further blame information.

Agreed.

May be this way also allows to do queries for deleted lines by making the callback stop when he finds a deleted line in a given range.



Best regards

Markus Schaber

(This email was sent from a mobile device...)

CODESYS® a trademark of 3S-Smart Software Solutions GmbH

Inspiring Automation Solutions
________________________________
3S-Smart Software Solutions GmbH
Dipl.-Inf. Markus Schaber | Product Development Core Technology
Memminger Str. 151 | 87439 Kempten | Germany
Tel. +49-831-54031-979 | Fax +49-831-54031-50

E-Mail: m.schaber@codesys.com | Web: codesys.com
CODESYS internet forum: forum.codesys.com

Managing Directors: Dipl.Inf. Dieter Hess, Dipl.Inf. Manfred Werner | Trade register: Kempten HRB 6186 | Tax ID No.: DE 167014915

________________________________________