On Fri, Dec 10, 2010 at 14:29, Daniel Shahaf <d...@daniel.shahaf.name> wrote: >> >> + /** How this property entered the node tree: 'A'dd, 'D'elete, >> >> 'R'eplace */ >> >> + char action; >> > >> > 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-479d37995...@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. >> >> 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. >> > > (will reply later) > >> >> + /** The name of the property */ >> >> + const char *name; >> > >> > Where is the value of the property? How to get it? >> >> The idea was that the struct should indicate changes to properties, >> not their values. In the same way that svn_repos_node_t shows changes, >> not node content. >> > > Flawed analogy: we never store node content in memory, but we do have > all property values in memory, so the cost is just to add an > svn_string_t * member to the struct. > > My question was how would callers get the value if they cared about it. > i.e., I assume that whoever calls this function already has a property > hash (or, at least, an fs object) available that they can get the > property values from?
I agree that it is simple to add the value to the struct, but in svnlook (which is the only current in-tree user of this api) I can't see any need to get the value. How a caller would get the property value if they indeed cared about it is outside my knowledge of the api. Perhaps svn_repos_node_editor() is not the editor to use if you want the property value? >> >> + /** Pointer to the next sibling property */ >> >> + struct svn_repos_node_prop_t *sibling; >> >> + >> > >> > You use a linked list. How about using apr_array_header_t *? Or a hash >> > of (prop_name -> struct)? >> >> I guess anyone of those would work, but the reason I went for a linked >> list was that svn_repos_node_t did that and I wanted them to be >> similar. >> > > Hmm. I haven't seen that struct before: svn_repos_node_t indeed uses an > explicit tree structure. > > What do you think will be more useful to consumers of the API? A hash > allows both random access and iteration --- do APR arrays or linked > lists have advantages that outweigh that? A hash seemed like overkill, but if you think it is better I have no problem using that. // Erik -- Erik Johansson Home Page: http://ejohansson.se/ PGP Key: http://ejohansson.se/erik.asc
