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...@elego.de> on 2013/06/13 10:35:46 UTC
Kidney blame's behaviour and edge cases
Definition: "kidney blame" == "blame -r N:M" with M<N. Currently it is
an error (raised by svn_client_blame5()).
By and large, it should do exactly what 'blame -r M:N' does: walk the
revisions from "before-the-colon revision" to "after-the-colon revision"
and then print the contents of the "after the colon" revision, with each
line annotated by the revnum of the "rightmost" (that is: nearest to the
revnum on the right of the colon) revision within the range that added
that line. In other words, if
(iota@4, kappa@1), (iota@3, kappa@2), (iota@2, kappa@3), (iota@1, kappa@4)
are pairs of byte-for-byte-identical files, then 'blame -r 1:4 kappa'
and 'blame -r 4:1 iota' will output the same thing.
Currently, the non-kidney blame checks one revision before the start ---
that is, 'blame -r M:N' actually calls svn_ra_get_file_revs2(start=M-1,
end=N). The purpose of this is to differentiate "lines added in rN"
from "lines present since before rN". That makes 'blame -r M:N'
correspond to 'diff -r M-1:N' or 'diff -c M:N', in that it also shows
changes made _by_ rM, not only _since_ rM.
That behaviour cannot easily replicated for the -r N:M case since it's
not possible to cheaply find the "next interesting revision" after rN,
but in any case I think it shouldn't be: 'blame -r M:N' should not have
automagically decremented M --- instead, if the user had wanted to see
what lines were added in rM, as opposed to before it, the user should
have specified M-1 as the start revision. That way, 'blame -r M:N'
would be consistent with 'diff -r M:N'.
So I suggest that blame -r N:M not try to find the "next change after
rN" (that's the analogous thing to what "decrement M" is in the '-r M:N'
case). That means -r M:N and -r N:M are assymetrical, and I propose we
fix that by making -r M:N not decrement M --- which we can probably only
do in 2.0.
---
Another issue: what should 'blame -r 3:3' do? Currently it is allowed,
and prints '-' for lines added before r3 and '3' for lines added in r3.
I am not sure whether that is intentional / by design, or just an
accident of the fact that whoever added the 'end_rev < start_rev' check
should have used the broader condition 'end_rev <= start_rev' instead.
(See r7438 <-> http://svn.apache.org/r847512).
It seems to me it should ideally print '3' for every line, and the user
should pass '-r 2:3' if he wants to distinguish "added in r3" from
"added before r3". It would be easy to preserve the current behaviour,
though, of printing '-' rather than '2' (where '2' here is the youngest
change to that line, for lines added before r3).
---
Cheers,
Daniel
Re: Kidney blame's behaviour and edge cases
Posted by Daniel Shahaf <da...@elego.de>.
Julian Foad wrote on Fri, Feb 20, 2015 at 20:15:29 +0000:
> I (Julian Foad) wrote:
> > issue #4565 "reverse blame, aka kidney blame"
> > [...] I want to see:
> >
> > * The first revision in which the line was changed (or deleted) after
> > r1400000.
>
> The following help text explains how I think it should behave:
>
> [[[
> blame (praise, annotate, ann): Show when each line of a file was last (or
> next) changed.
> usage: blame [-rM:N] TARGET[@REV]...
>
> Annotate each line of a file with the revision number and author of the
> last change (or optionally the next change) to that line.
>
> With no revision range (same as -r0:REV), or with '-r M:N' where M < N,
> annotate each line that is present in revision N of the file,
> with the last revision at or before rN that changed or added the line,
> looking back no further than rM.
>
> With a reverse revision range '-r M:N' where M > N,
> annotate each line that is present in revision N of the file,
> with the NEXT revision AFTER rN that changed or DELETED the line,
> looking forward no further than rM.
>
> Write the annotated result to standard output.
>
> If specified, REV determines in which revision the target is first
> looked up.
> ]]]
>
> Makes sense?
As discussed on IRC: yes, makes sense. Thanks to you and to Bert for
fixing this!
Re: Kidney blame's behaviour and edge cases
Posted by Julian Foad <ju...@btopenworld.com>.
Bert fixed the code to work like this, in r1661208.
I committed help text, much like I pasted here, in r1661211.
- Julian
I (Julian Foad) wrote:
> I (Julian Foad) wrote:
>> issue #4565 "reverse blame, aka kidney blame"
>> [...] I want to see:
>>
>> * The first revision in which the line was changed (or deleted) after
>> r1400000.
>
> The following help text explains how I think it should behave:
>
> [[[
> blame (praise, annotate, ann): Show when each line of a file was last (or
> next) changed.
> usage: blame [-rM:N] TARGET[@REV]...
>
> Annotate each line of a file with the revision number and author of the
> last change (or optionally the next change) to that line.
>
> With no revision range (same as -r0:REV), or with '-r M:N' where M
> < N,
> annotate each line that is present in revision N of the file,
> with the last revision at or before rN that changed or added the line,
> looking back no further than rM.
>
> With a reverse revision range '-r M:N' where M > N,
> annotate each line that is present in revision N of the file,
> with the NEXT revision AFTER rN that changed or DELETED the line,
> looking forward no further than rM.
>
> Write the annotated result to standard output.
>
> If specified, REV determines in which revision the target is first
> looked up.
> ]]]
>
> Makes sense?
>
> - Julian
>
Re: Kidney blame's behaviour and edge cases
Posted by Julian Foad <ju...@btopenworld.com>.
I (Julian Foad) wrote:
> issue #4565 "reverse blame, aka kidney blame"
> [...] I want to see:
>
> * The first revision in which the line was changed (or deleted) after
> r1400000.
The following help text explains how I think it should behave:
[[[
blame (praise, annotate, ann): Show when each line of a file was last (or
next) changed.
usage: blame [-rM:N] TARGET[@REV]...
Annotate each line of a file with the revision number and author of the
last change (or optionally the next change) to that line.
With no revision range (same as -r0:REV), or with '-r M:N' where M < N,
annotate each line that is present in revision N of the file,
with the last revision at or before rN that changed or added the line,
looking back no further than rM.
With a reverse revision range '-r M:N' where M > N,
annotate each line that is present in revision N of the file,
with the NEXT revision AFTER rN that changed or DELETED the line,
looking forward no further than rM.
Write the annotated result to standard output.
If specified, REV determines in which revision the target is first
looked up.
]]]
Makes sense?
- Julian
Re: Kidney blame's behaviour and edge cases
Posted by Julian Foad <ju...@btopenworld.com>.
I filed issue #4565 "reverse blame, aka kidney blame" to track this enhancement, because I think it is useful to have an issue to coordinate any change we make in a release.
It currently doesn't behave how I think it should. Try
svn blame -r1600000:1400000 ^/subversion/trunk/subversion/svn/svn.c@1660000
Notice that nearly every line of the output is annotated as r1591022. This file was indeed changed in r1591022, but only three lines were changed: one added, two deleted.
There may be an end-of-revision-range problem here, where all lines otherwise untouched are given a revision number but should instead be marked as "-". But if that's happening, it's still not the main problem here.
The main problem seems to be that it's annotating each line of the old file (svn.c@1400000) with:
* The last (youngest) revision in which the file was changed *before* the first revision in which this line was changed (or deleted) after r1400000.
To me, that's not a good output. I want to see:
* The first revision in which the line was changed (or deleted) after r1400000.
To me, that's what would correspond to the logical 'reverse' of the normal blame.
I note that the original email in this thread [1] did not include a description of the intended output in behavioural terms, and I think the description it included in terms of 'iota and kappa' was faulty.
Anyone else tried it? What do you think?
- Julian
[1]
2013-06-13, archived at e.g. http://mail-archives.apache.org/mod_mbox/subversion-dev/201306.mbox/%3C20130613083546.GA2773@tarsus.local2%3E and http://svn.haxx.se/dev/archive-2013-06/0230.shtml
RE: Kidney blame's behaviour and edge cases
Posted by Bert Huijben <be...@qqmail.nl>.
> -----Original Message-----
> From: Daniel Shahaf [mailto:danielsh@elego.de]
> Sent: donderdag 13 juni 2013 10:36
> To: dev@subversion.apache.org
> Subject: Kidney blame's behaviour and edge cases
>
> Another issue: what should 'blame -r 3:3' do? Currently it is allowed,
> and prints '-' for lines added before r3 and '3' for lines added in r3.
> I am not sure whether that is intentional / by design, or just an
> accident of the fact that whoever added the 'end_rev < start_rev' check
> should have used the broader condition 'end_rev <= start_rev' instead.
> (See r7438 <-> http://svn.apache.org/r847512).
>
> It seems to me it should ideally print '3' for every line, and the user
> should pass '-r 2:3' if he wants to distinguish "added in r3" from
> "added before r3". It would be easy to preserve the current behaviour,
> though, of printing '-' rather than '2' (where '2' here is the youngest
> change to that line, for lines added before r3).
I would call this intentional, and would really like to keep this behavior.
I certainly don't call it a bug, as this is what makes it possible to know
that you need a larger range for blaming to obtain more info.
Bert
Re: Kidney blame's behaviour and edge cases
Posted by Prabhu Gnana Sundar <pr...@collab.net>.
Johan Corveleyn <jc...@gmail.com> wrote:
>On Thu, Jun 13, 2013 at 6:10 PM, Doug Robinson
><do...@wandisco.com> wrote:
>> Daniel:
>>
>> I think that simply enabling M<N (where it is now an error) will
>create the
>> situation where the user makes a mistake, gets something they don't
>expect
>> and tries to interpret it based on their desire - leading to
>confusion. I
>> believe M<N should still be an error. A new option (--reverse ?)
>should be
>> required to make it clear that the user wants the reverse blame walk.
>
>I agree. I think it would be better to make the reverseness explicit
>in the UI. When running blame, users are not used to think about the
>order of their -r arguments.
>
>But then I'm wondering: should 'svn blame --reverse -r1:5' give an
>error, just like 'svn blame -r5:1' does? Or should it silently swap
>the revnum args? Hmmm
I would say, silently swap and blame because to me that seems more sensible...
--
prabhu
--
Sent from my Android phone with K-9 Mail. Please excuse my brevity.
Re: Kidney blame's behaviour and edge cases
Posted by Johan Corveleyn <jc...@gmail.com>.
On Thu, Jun 13, 2013 at 6:10 PM, Doug Robinson
<do...@wandisco.com> wrote:
> Daniel:
>
> I think that simply enabling M<N (where it is now an error) will create the
> situation where the user makes a mistake, gets something they don't expect
> and tries to interpret it based on their desire - leading to confusion. I
> believe M<N should still be an error. A new option (--reverse ?) should be
> required to make it clear that the user wants the reverse blame walk.
I agree. I think it would be better to make the reverseness explicit
in the UI. When running blame, users are not used to think about the
order of their -r arguments.
But then I'm wondering: should 'svn blame --reverse -r1:5' give an
error, just like 'svn blame -r5:1' does? Or should it silently swap
the revnum args? Hmmm
(BTW, I think the reverse (kidney) blame will always remain a rarely
used usecase. It's really kind of advanced, just thinking about it
...)
--
Johan
Re: Kidney blame's behaviour and edge cases
Posted by Johan Corveleyn <jc...@gmail.com>.
On Fri, Jun 14, 2013 at 11:36 AM, Daniel Shahaf <da...@elego.de> wrote:
> Johan Corveleyn wrote on Fri, Jun 14, 2013 at 11:16:06 +0200:
>> On Fri, Jun 14, 2013 at 11:00 AM, Daniel Shahaf <da...@elego.de> wrote:
>> > Doug Robinson wrote on Thu, Jun 13, 2013 at 12:10:49 -0400:
>> >> Daniel:
>> >>
>> >> I think that simply enabling M<N (where it is now an error) will create the
>> >> situation where the user makes a mistake, gets something they don't expect
>> >> and tries to interpret it based on their desire - leading to confusion. I
>> >> believe M<N should still be an error. A new option (--reverse ?) should be
>> >> required to make it clear that the user wants the reverse blame walk.
>> >
>> > Sorry, disagree.
>> >
>> > diff -r 1:5 != diff -r 5:1
>> > log -r 1:5 != log -r 5:1
>> > merge -r 4:5 != merge -r 5:4
>> >
>> > With all that in mind, I still think that making 'blame -r 5:4' and
>> > 'blame -r 4:5' do different things is the correct course of action.
>> >
>>
>> Okay, I don't feel strongly about this. My only "argument" was that
>> people are not used to thinking about the order of rev args when using
>> blame. But that doesn't mean they can't get used to it ...
>
> Do people use blame -r M:N at all? I would expect that 'svn blame file'
> and 'svn blame -r N file' / 'svn blame file@rN' are more popular
> invocations.
Good point.
Okay, +1 on just letting -r5:4 do the reverse thing without anything more.
GUI's can obviously do other things in their dialog windows to make
things clear to users, if they want to. But for the commandline I
think your proposal is fine.
--
Johan
Re: Kidney blame's behaviour and edge cases
Posted by Daniel Shahaf <da...@elego.de>.
Johan Corveleyn wrote on Fri, Jun 14, 2013 at 11:16:06 +0200:
> On Fri, Jun 14, 2013 at 11:00 AM, Daniel Shahaf <da...@elego.de> wrote:
> > Doug Robinson wrote on Thu, Jun 13, 2013 at 12:10:49 -0400:
> >> Daniel:
> >>
> >> I think that simply enabling M<N (where it is now an error) will create the
> >> situation where the user makes a mistake, gets something they don't expect
> >> and tries to interpret it based on their desire - leading to confusion. I
> >> believe M<N should still be an error. A new option (--reverse ?) should be
> >> required to make it clear that the user wants the reverse blame walk.
> >
> > Sorry, disagree.
> >
> > diff -r 1:5 != diff -r 5:1
> > log -r 1:5 != log -r 5:1
> > merge -r 4:5 != merge -r 5:4
> >
> > With all that in mind, I still think that making 'blame -r 5:4' and
> > 'blame -r 4:5' do different things is the correct course of action.
> >
>
> Okay, I don't feel strongly about this. My only "argument" was that
> people are not used to thinking about the order of rev args when using
> blame. But that doesn't mean they can't get used to it ...
Do people use blame -r M:N at all? I would expect that 'svn blame file'
and 'svn blame -r N file' / 'svn blame file@rN' are more popular
invocations.
Re: Kidney blame's behaviour and edge cases
Posted by Johan Corveleyn <jc...@gmail.com>.
On Fri, Jun 14, 2013 at 1:33 PM, Daniel Shahaf <da...@elego.de> wrote:
> Johan Corveleyn wrote on Fri, Jun 14, 2013 at 11:16:06 +0200:
>> On Fri, Jun 14, 2013 at 11:00 AM, Daniel Shahaf <da...@elego.de> wrote:
>> > Doug Robinson wrote on Thu, Jun 13, 2013 at 12:10:49 -0400:
>> >> Daniel:
>> >>
>> >> I think that simply enabling M<N (where it is now an error) will create the
>> >> situation where the user makes a mistake, gets something they don't expect
>> >> and tries to interpret it based on their desire - leading to confusion. I
>> >> believe M<N should still be an error. A new option (--reverse ?) should be
>> >> required to make it clear that the user wants the reverse blame walk.
>> >
>> > Sorry, disagree.
>> >
>> > diff -r 1:5 != diff -r 5:1
>> > log -r 1:5 != log -r 5:1
>> > merge -r 4:5 != merge -r 5:4
>> >
>> > With all that in mind, I still think that making 'blame -r 5:4' and
>> > 'blame -r 4:5' do different things is the correct course of action.
>> >
>>
>> Okay, I don't feel strongly about this. My only "argument" was that
>> people are not used to thinking about the order of rev args when using
>> blame. But that doesn't mean they can't get used to it ...
>
> Implemented in r1493027. No API changes are involved --- this simply
> makes 'blame -r 5:4' do something instead of raising an error
> immediately --- so I wonder if we should backport it.
>
> I'll go ahead and put it in STATUS towards 1.8.1, if people prefer a
> backport not to happen they can go ahead and cast -0 votes and continue
> discussion here.
There are still two problems with the implementation you committed in r1493027:
With 'svn blame M:N' where M>N
1) It's using the N as peg revision, while it should use M (but you're
already working on that).
2) If N is before the item existed, I get:
svn: E195012: Unable to find repository location for
'svn://localhost/path/to/file.txt' in revision 1
It would be nice if you could just blame up to the oldest revision
close to 'end' where the item still existed.
--
Johan
Re: Kidney blame's behaviour and edge cases
Posted by Daniel Shahaf <da...@elego.de>.
Johan Corveleyn wrote on Fri, Jun 14, 2013 at 11:16:06 +0200:
> On Fri, Jun 14, 2013 at 11:00 AM, Daniel Shahaf <da...@elego.de> wrote:
> > Doug Robinson wrote on Thu, Jun 13, 2013 at 12:10:49 -0400:
> >> Daniel:
> >>
> >> I think that simply enabling M<N (where it is now an error) will create the
> >> situation where the user makes a mistake, gets something they don't expect
> >> and tries to interpret it based on their desire - leading to confusion. I
> >> believe M<N should still be an error. A new option (--reverse ?) should be
> >> required to make it clear that the user wants the reverse blame walk.
> >
> > Sorry, disagree.
> >
> > diff -r 1:5 != diff -r 5:1
> > log -r 1:5 != log -r 5:1
> > merge -r 4:5 != merge -r 5:4
> >
> > With all that in mind, I still think that making 'blame -r 5:4' and
> > 'blame -r 4:5' do different things is the correct course of action.
> >
>
> Okay, I don't feel strongly about this. My only "argument" was that
> people are not used to thinking about the order of rev args when using
> blame. But that doesn't mean they can't get used to it ...
Implemented in r1493027. No API changes are involved --- this simply
makes 'blame -r 5:4' do something instead of raising an error
immediately --- so I wonder if we should backport it.
I'll go ahead and put it in STATUS towards 1.8.1, if people prefer a
backport not to happen they can go ahead and cast -0 votes and continue
discussion here.
Re: Kidney blame's behaviour and edge cases
Posted by Johan Corveleyn <jc...@gmail.com>.
On Fri, Jun 14, 2013 at 11:00 AM, Daniel Shahaf <da...@elego.de> wrote:
> Doug Robinson wrote on Thu, Jun 13, 2013 at 12:10:49 -0400:
>> Daniel:
>>
>> I think that simply enabling M<N (where it is now an error) will create the
>> situation where the user makes a mistake, gets something they don't expect
>> and tries to interpret it based on their desire - leading to confusion. I
>> believe M<N should still be an error. A new option (--reverse ?) should be
>> required to make it clear that the user wants the reverse blame walk.
>
> Sorry, disagree.
>
> diff -r 1:5 != diff -r 5:1
> log -r 1:5 != log -r 5:1
> merge -r 4:5 != merge -r 5:4
>
> With all that in mind, I still think that making 'blame -r 5:4' and
> 'blame -r 4:5' do different things is the correct course of action.
>
Okay, I don't feel strongly about this. My only "argument" was that
people are not used to thinking about the order of rev args when using
blame. But that doesn't mean they can't get used to it ...
--
Johan
Re: Kidney blame's behaviour and edge cases
Posted by Daniel Shahaf <da...@elego.de>.
Daniel Shahaf wrote on Fri, Jun 14, 2013 at 11:18:35 +0200:
> Prabhu wrote on Fri, Jun 14, 2013 at 14:33:57 +0530:
> > On 06/14/2013 02:30 PM, Daniel Shahaf wrote:
> > >Doug Robinson wrote on Thu, Jun 13, 2013 at 12:10:49 -0400:
> > >>Daniel:
> > >>
> > >>I think that simply enabling M<N (where it is now an error) will create the
> > >>situation where the user makes a mistake, gets something they don't expect
> > >>and tries to interpret it based on their desire - leading to confusion. I
> > >>believe M<N should still be an error. A new option (--reverse ?) should be
> > >>required to make it clear that the user wants the reverse blame walk.
> > >Sorry, disagree.
> > >
> > >diff -r 1:5 != diff -r 5:1
> > >log -r 1:5 != log -r 5:1
> > >merge -r 4:5 != merge -r 5:4
> > >
> > >With all that in mind, I still think that making 'blame -r 5:4' and
> > >'blame -r 4:5' do different things is the correct course of action.
> >
> >
> > Yeah, perhaps 'blame -r 5:4' and 'blame -r4:5 --reverse' should do
> > the same ?
>
> If you do that, why not allow 'svn merge -c 5 --reverse', 'svn diff -c 5
> --reverse', etc as well?
And, of course, 'svn merge -c-5 --reverse', which is going to be
confusing to someone regardless of whether you define it as -r4:5 or
as -r5:4 ...
Re: Kidney blame's behaviour and edge cases
Posted by Daniel Shahaf <da...@elego.de>.
Prabhu wrote on Fri, Jun 14, 2013 at 14:33:57 +0530:
> On 06/14/2013 02:30 PM, Daniel Shahaf wrote:
> >Doug Robinson wrote on Thu, Jun 13, 2013 at 12:10:49 -0400:
> >>Daniel:
> >>
> >>I think that simply enabling M<N (where it is now an error) will create the
> >>situation where the user makes a mistake, gets something they don't expect
> >>and tries to interpret it based on their desire - leading to confusion. I
> >>believe M<N should still be an error. A new option (--reverse ?) should be
> >>required to make it clear that the user wants the reverse blame walk.
> >Sorry, disagree.
> >
> >diff -r 1:5 != diff -r 5:1
> >log -r 1:5 != log -r 5:1
> >merge -r 4:5 != merge -r 5:4
> >
> >With all that in mind, I still think that making 'blame -r 5:4' and
> >'blame -r 4:5' do different things is the correct course of action.
>
>
> Yeah, perhaps 'blame -r 5:4' and 'blame -r4:5 --reverse' should do
> the same ?
If you do that, why not allow 'svn merge -c 5 --reverse', 'svn diff -c 5
--reverse', etc as well?
Does "There should be one-- and preferably only one --obvious way to do it."
not apply here?
Re: Kidney blame's behaviour and edge cases
Posted by Prabhu <pr...@collab.net>.
On 06/14/2013 02:30 PM, Daniel Shahaf wrote:
> Doug Robinson wrote on Thu, Jun 13, 2013 at 12:10:49 -0400:
>> Daniel:
>>
>> I think that simply enabling M<N (where it is now an error) will create the
>> situation where the user makes a mistake, gets something they don't expect
>> and tries to interpret it based on their desire - leading to confusion. I
>> believe M<N should still be an error. A new option (--reverse ?) should be
>> required to make it clear that the user wants the reverse blame walk.
> Sorry, disagree.
>
> diff -r 1:5 != diff -r 5:1
> log -r 1:5 != log -r 5:1
> merge -r 4:5 != merge -r 5:4
>
> With all that in mind, I still think that making 'blame -r 5:4' and
> 'blame -r 4:5' do different things is the correct course of action.
Yeah, perhaps 'blame -r 5:4' and 'blame -r4:5 --reverse' should do the
same ?
--
Prabhu
Re: Kidney blame's behaviour and edge cases
Posted by Daniel Shahaf <da...@elego.de>.
Doug Robinson wrote on Thu, Jun 13, 2013 at 12:10:49 -0400:
> Daniel:
>
> I think that simply enabling M<N (where it is now an error) will create the
> situation where the user makes a mistake, gets something they don't expect
> and tries to interpret it based on their desire - leading to confusion. I
> believe M<N should still be an error. A new option (--reverse ?) should be
> required to make it clear that the user wants the reverse blame walk.
Sorry, disagree.
diff -r 1:5 != diff -r 5:1
log -r 1:5 != log -r 5:1
merge -r 4:5 != merge -r 5:4
With all that in mind, I still think that making 'blame -r 5:4' and
'blame -r 4:5' do different things is the correct course of action.
Cheers,
Daniel
Re: Kidney blame's behaviour and edge cases
Posted by Doug Robinson <do...@wandisco.com>.
Daniel:
I think that simply enabling M<N (where it is now an error) will create the
situation where the user makes a mistake, gets something they don't expect
and tries to interpret it based on their desire - leading to confusion. I
believe M<N should still be an error. A new option (--reverse ?) should be
required to make it clear that the user wants the reverse blame walk.
Thank you.
Doug
On Thu, Jun 13, 2013 at 4:35 AM, Daniel Shahaf <da...@elego.de> wrote:
> Definition: "kidney blame" == "blame -r N:M" with M<N. Currently it is
> an error (raised by svn_client_blame5()).
>
> By and large, it should do exactly what 'blame -r M:N' does: walk the
> revisions from "before-the-colon revision" to "after-the-colon revision"
> and then print the contents of the "after the colon" revision, with each
> line annotated by the revnum of the "rightmost" (that is: nearest to the
> revnum on the right of the colon) revision within the range that added
> that line. In other words, if
> (iota@4, kappa@1), (iota@3, kappa@2), (iota@2, kappa@3), (iota@1, kappa@4)
> are pairs of byte-for-byte-identical files, then 'blame -r 1:4 kappa'
> and 'blame -r 4:1 iota' will output the same thing.
>
> Currently, the non-kidney blame checks one revision before the start ---
> that is, 'blame -r M:N' actually calls svn_ra_get_file_revs2(start=M-1,
> end=N). The purpose of this is to differentiate "lines added in rN"
> from "lines present since before rN". That makes 'blame -r M:N'
> correspond to 'diff -r M-1:N' or 'diff -c M:N', in that it also shows
> changes made _by_ rM, not only _since_ rM.
>
> That behaviour cannot easily replicated for the -r N:M case since it's
> not possible to cheaply find the "next interesting revision" after rN,
> but in any case I think it shouldn't be: 'blame -r M:N' should not have
> automagically decremented M --- instead, if the user had wanted to see
> what lines were added in rM, as opposed to before it, the user should
> have specified M-1 as the start revision. That way, 'blame -r M:N'
> would be consistent with 'diff -r M:N'.
>
> So I suggest that blame -r N:M not try to find the "next change after
> rN" (that's the analogous thing to what "decrement M" is in the '-r M:N'
> case). That means -r M:N and -r N:M are assymetrical, and I propose we
> fix that by making -r M:N not decrement M --- which we can probably only
> do in 2.0.
>
> ---
>
> Another issue: what should 'blame -r 3:3' do? Currently it is allowed,
> and prints '-' for lines added before r3 and '3' for lines added in r3.
> I am not sure whether that is intentional / by design, or just an
> accident of the fact that whoever added the 'end_rev < start_rev' check
> should have used the broader condition 'end_rev <= start_rev' instead.
> (See r7438 <-> http://svn.apache.org/r847512).
>
> It seems to me it should ideally print '3' for every line, and the user
> should pass '-r 2:3' if he wants to distinguish "added in r3" from
> "added before r3". It would be easy to preserve the current behaviour,
> though, of printing '-' rather than '2' (where '2' here is the youngest
> change to that line, for lines added before r3).
>
> ---
>
> Cheers,
>
> Daniel
>
--
Douglas B. Robinson | *Senior Product Manager*
WANdisco // *Non-Stop Data*
t. 925-396-1125
e. doug.robinson@wandisco.com
--
THIS MESSAGE AND ANY ATTACHMENTS ARE CONFIDENTIAL, PROPRIETARY, AND MAY BE
PRIVILEGED. If this message was misdirected, WANdisco, Inc. and its
subsidiaries, ("WANdisco") does not waive any confidentiality or privilege.
If you are not the intended recipient, please notify us immediately and
destroy the message without disclosing its contents to anyone. Any
distribution, use or copying of this e-mail or the information it contains
by other than an intended recipient is unauthorized. The views and
opinions expressed in this e-mail message are the author's own and may not
reflect the views and opinions of WANdisco, unless the author is authorized
by WANdisco to express such views or opinions on its behalf. All email
sent to or from this address is subject to electronic storage and review by
WANdisco. Although WANdisco operates anti-virus programs, it does not
accept responsibility for any damage whatsoever caused by viruses being
passed.
Re: Kidney blame's behaviour and edge cases
Posted by Daniel Shahaf <da...@elego.de>.
Branko Čibej wrote on Thu, Jun 13, 2013 at 10:54:47 +0200:
> On 13.06.2013 10:35, Daniel Shahaf wrote:
> > It seems to me it should ideally print '3' for every line, and the user
> > should pass '-r 2:3' if he wants to distinguish "added in r3" from
> > "added before r3". It would be easy to preserve the current behaviour,
> > though, of printing '-' rather than '2' (where '2' here is the youngest
> > change to that line, for lines added before r3).
>
> It strikes me that what you're really looking for is either
>
(I assume you meant M<N throughout your email)
> -rN:M -> [N, M-1],
>
> which would make the two blame variants symmetrical, or
>
It would make them symmetrical in that they both extend the range one
revision in the "towards older" direction. I think that's not the
symmetry we need. My kappa/iota example outlines the kind of symmetry
I'm expecting.
> -rN:M -> [N+1,M]
>
> as you don't really have to find "the next interesting change" -- you
> only have to make sure the first bound is inclusive, as with -c, rather
> than exclusive, as with -r.
If you want to make the first bound inclusive, you need to extend the
range in the opposite direction, into [next-younger-interesting-revision, M].
(And then you run into the problem that in M:N you can give M=0, but
there's no equivalent for that in the N:M case.)
> That would also imply
>
> -cN:M -> [N+1,M]
>
> and I'm not sure we currently do it that way.
'svn blame' does not take the '-c' option, only '-r'.
Re: Kidney blame's behaviour and edge cases
Posted by Branko Čibej <br...@wandisco.com>.
On 13.06.2013 10:35, Daniel Shahaf wrote:
> It seems to me it should ideally print '3' for every line, and the user
> should pass '-r 2:3' if he wants to distinguish "added in r3" from
> "added before r3". It would be easy to preserve the current behaviour,
> though, of printing '-' rather than '2' (where '2' here is the youngest
> change to that line, for lines added before r3).
It strikes me that what you're really looking for is either
-rN:M -> [N, M-1],
which would make the two blame variants symmetrical, or
-rN:M -> [N+1,M]
as you don't really have to find "the next interesting change" -- you
only have to make sure the first bound is inclusive, as with -c, rather
than exclusive, as with -r. That would also imply
-cN:M -> [N+1,M]
and I'm not sure we currently do it that way.
-- Brane
--
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. brane@wandisco.com