Re: [PATCH] support printing changed properties in svnlook

On Fri, Dec 10, 2010 at 15:08, Daniel Shahaf <d.s_at_daniel.shahaf.name> wrote:
> [ I'm somewhat confused about this issue. ]
>
> Erik Johansson wrote on Thu, Dec 09, 2010 at 21:41:38 +0100:
>> On Wed, Dec 8, 2010 at 23:22, Daniel Shahaf <d.s_at_daniel.shahaf.name> wrote:
>> > Erik Johansson wrote on Wed, Dec 08, 2010 at 17:17:47 +0100:
>> >> +  /** How this property entered the node tree: 'A'dd, 'D'elete, 'R'eplace */
>> >> +  char action;
>
> I should have caught this earlier, but "replaced" doesn't make sense
> for properties.  (A node can be replaced by another node at the same
> path, but for a fixed node properties can be added/modified/removed but
> not replaced.)

I'll change it to 'A'dd, 'D'elete or 'U'pdate then?

>> >
>> > This is copied from svn_repos_node_t->action.  There was recently
>> > a question about that field:
>> > http://mid.gmane.org/3ABD28AA-A2FC-4D7D-A502-479D37995DB9@orcaware.com
>> >
>> > So, that asks whether 'C'hanged is a valid answer to the question that
>> > ->action is meant to answer.  I'll also ask how this interacts with node
>> > changes: for example; if r5 replaces-with-history a node that has
>> > 'fooprop' set with another node that also has 'fooprop' set, what would
>> > the value of 'action' be?
>>
>> What about this:
>> When a node is deleted all the properties it had are listed in
>> mod_prop with action D.
>>
>> When a node is added-with-history all the properties the source had
>> are listed in mod_prop with action A and a new flag copyfrom = TRUE.
>>
> There is already a copyfrom indication in svn_repos_node_t->copyfrom_rev,
> I don't think we need another one.

I was thinking that the copyfrom flag would be useful in the case
where you copy a node with a propery and add another property before
committing. Here you would have two property with action A and one
would have copyfrom = TRUE.

>> A replace-with-history will result in two repos_nodes, each having a
>> mod_prop list. If the same property exists in both it means it has
>> been replaced.
>>
> Two nodes doesn't sound like a good idea; one node marked as 'replaced'
> should suffice.

In the current api a replaced node is represented by two nodes. Should
we change this? Should svnlook stay back-wards compatible and print a
replaced node twice, once as Deleted and once as Added?

> In case of a replacement, do we want the API to provide the replaced
> node's properties? (at least their names)

It would be pretty simple if there are two separate nodes for a
replaced node. The deleted node will have all the properties it had
marked as Deleted. The new node will have its properties marked as
appropiate.

// Erik

-- 
Erik Johansson
Home Page: http://ejohansson.se/
PGP Key: http://ejohansson.se/erik.asc