subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Lamar Goddard <lamargodd...@gmail.com>
Subject Re: [PATCH] issue #3292: fix for 'svnlook diff' regression: doesn't print added dirs
Date Thu, 10 Jun 2010 01:10:29 GMT
Oh, I don't think my reply on the other thread made it to the dev
list, hopefully this one will.

On Wed, Jun 9, 2010 at 1:11 AM, Stefan Sperling <stsp@elego.de> wrote:
> On Fri, Jun 04, 2010 at 06:02:52PM -0700, Lamar Goddard wrote:
>> [[[
>> Fixes issue #3292: Output svnlook diff content for copied/moved directories
>>
>> * subversion/svnlook/main.c
>>   (print_diff_tree): If diff isn't outputted but header has content,
>> output the header.
>> ]]]
>
> Hi Lamar,
>
> thanks for the patch. It is perferectly fine technically,
> but I'm unsure about how we (Subversion developers) want 'svnlook diff'
> to behave.
>
> svnlook diff in 1.4 behaves differently than svn diff does in trunk.
> svn diff never shows anything for directories (copied or not), it only shows
> diffs for files. Do we want svn diff and svnlook diff behave the same way?
> If so, maybe the 1.5/1.6 behaviour is correct and the 1.4 behaviour was wrong?


My 2¢:

I'm not sure svnlook diff should match svn diff's behavior.

>From what I understand there's no equivalent svnlook command to svn
log -v or svn info so there's currently no way for a well behaved hook
script to determine where a directory copy originated.  svn log -v
provides that information for revisions in the repository and svn info
does the same for working copy changes.


> Assuming we do want to show a Copied: header for copied directories in
> svnlook diff, why aren't we also showing an Added: header for added
> directories, and a Deleted: header for deleted directories?


I don't think there's a need for an Added or Deleted header because
those are isolated actions that are self contained in the one path.
Copied is different because it relates to more than one path.  Knowing
the destination only gives you a piece of the puzzle.  It's the
difference in the output between (current):


Property changed on: branches/foo
___________________________________________________________________
Added: svn:mergeinfo
  + /branches/bar:1-10


and (patched):


Copied: branches/foo (from rev 20,
branches/random_path/that_you_will/never_guess)

Property changed on: branches/foo
___________________________________________________________________
Added: svn:mergeinfo
  + /branches/bar:1-10


In the first case you only know the destination but in the second you
also know the source.  Diff may not be the correct function but it's
where this information used to be provided for directories and still
is for files.


-- 
Lamar Goddard
lamargoddard@gmail.com

Mime
View raw message