Paul burba added and shortly after reverted (as no longer used) code to separate these steps. You can probably revive that code.
Bert From: Daniel Shahaf Sent: 14/06/2013 17:49 To: Bert Huijben Cc: Johan Corveleyn; Subversion Development Subject: Re: Kidney blame's behaviour and edge cases Daniel Shahaf wrote on Fri, Jun 14, 2013 at 17:41:18 +0200: > No typo, but that function doesn't help, since we don't have a revision > in which the file existed; the use-case is to do 'svn blame -r HEAD:0 > file' and have the 0 become the first revision in which the file > existed. > > It seem the fix is to use svn_client__repos_location_segments(), like > 'log' does. > > I am still not happy with the svn_client_blame5() patch I committed --- > specifically, with the way it opens at 'end' and then tries to move to > 'start' by re-doing the last part of svn_client__ra_session_from_path2(). > I think that part needs to be redone. I'm not sure how yet. > What I'd conceptually like to do is: 1 - Open RA session to repository root 2 - Resolve the two svn_opt_revision_t arguments to numbers; call the larger X 3 - Reparent to 'target@peg_revision -r X' 4 - Run blame But the current code doesn't separate (1) and (2). Daniel > Bert Huijben wrote on Fri, Jun 14, 2013 at 08:24:58 -0700: > > Svn_ra_get_deleted_rev() ? > > (could have a typo) > > > > Bert From: Daniel Shahaf > > Sent: 14/06/2013 17:11 > > To: Bert Huijben > > Cc: Johan Corveleyn; Subversion Development > > Subject: Re: Kidney blame's behaviour and edge cases > > Bert Huijben wrote on Fri, Jun 14, 2013 at 08:06:25 -0700: > > > I would guess 1 and twi are actually the same problem: no node found > > > via peg revision. > > > > > > Bert From: Johan Corveleyn > > > Sent: 14/06/2013 16:51 > > > To: Daniel Shahaf > > > Cc: Subversion Development > > > Subject: Re: Kidney blame's behaviour and edge cases > > > On Fri, Jun 14, 2013 at 1:33 PM, Daniel Shahaf <danie...@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 <danie...@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). > > > > > > > Should be fixed by r1493106. I'd welcome further review of that, I > > am unsure that the "open ra session to the other svn_opt_revision_t" > > part is idiomatic. > > > > > 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. > > > > Yeah, it would be nice if 'svn blame -r HEAD:1' just worked even for > > files added later. I can look into that, but not today :) It would > > be helpful if someone could point me to another place in the codebase > > that solves the same problem. > > > > Cheers, > > > > Daniel
