On Tue, Sep 9, 2008 at 8:15 AM, Karl Fogel <kfogel_at_red-bean.com> wrote:
> gstein_at_tigris.org writes:
>> --- trunk/subversion/libsvn_wc/wc_db.h
>> +++ trunk/subversion/libsvn_wc/wc_db.h
>> @@ -64,6 +69,15 @@ typedef enum svn_wc__db_kind_t {
>> } svn_wc__db_kind_t;
>>
>>
>> +typedef enum svn_wc__db_status_t {
>> + svn_wc__db_status_normal,
>> + svn_wc__db_status_added, /* ### no history. text_mod set to TRUE */
>> + svn_wc__db_status_moved, /* ### has history */
>> + svn_wc__db_status_copied, /* ### has history */
>> + svn_wc__db_status_deleted /* ### text_mod, prop_mod will be FALSE */
>> +} svn_wc__db_status_t;
>
> How to represent:
>
> - replaced (without history)
> - replaced (with history)
>
> ?
I was thinking that "replaced without history" is just an add. The
fact that it "shadows" a previously-deleted item is a different
question, and is answered by a new boolean named "base_shadowed" in
the _read_info() API. This is similar to copying or moving a node over
the top of another. We're talking about the status of what is there
now. The fact that something *was* there is orthogonal.
> Also, for the 'moved' and 'copied' enums, would they refer to the source
> or dest of the operation?
This is the status of the destination, just like our current interface.
But your point is good: what about the source of a move. In order for
this API to properly represent a move, the source is not "deleted",
but "moved away". I'll update the enum to clarify that situation.
Note: I plan to have the WC *support* these moves. Whether the callers
use it or not... we will need to ripple that out. For example, maybe
the client library would start to record moves, but when we send them
to the server, they just become add/delete pairs.
>...
> Okay, I understand the semantics now (this is svn_wc__db_base_move()),
> but I don't understand the motivation...
As above. To start supporting a first-order move concept. Then ripple
it further out.
>...
>> -svn-error_t *
>> +svn_error_t *
>
> Heh. Guess GCC would have caught that one :-).
Not would. Did. :-)
>> @@ -350,7 +364,7 @@ svn_wc__db_op_copy_url(svn_wc__db_t *db,
>> apr_pool_t *scratch_pool);
>>
>>
>> -/* ### props, children may be NULL
>> +/* ### props may be NULL. children must be known before calling.
>> *
>> * ### KFF: Okay, if children can be NULL here, then it should be
>> * ### able to be NULL up in svn_wc__db_base_add_directory().
>
> I didn't understand "children must be known before calling". Does it
> mean that children cannot be changed after calling or something?
The set cannot be changed as a whole [so specify the initial set up
front], but you can add/remove children through the other APIs (e.g.
op_add_file).
Primarily, this was a note/anwer to myself that you must specify
children rather than make it optional. NULL implies no children rather
than "don't care to specify at this time".
>...
>> + ### original_url will be NULL if this node is not copied/moved
>> + ### original_rev will be SVN_INVALID_REVNUM if this node is not copied/moved
>
> s/will/must/ ?
Well, these are OUT params, and MUST implies a requirement-for-action.
Doesn't fit very well here. I guess if you're talking about the
implementation... :-P
> I think it's fine to have lots of individual params. This is an
> internal interface, after all -- updating it when we add something new
> is easy, and the compiler won't let us forgot.
My intent is to allow NULL for any of the params indicating "I don't
care about that", so the implementation can optimize its behavior in
different ways if you are interested in just a subset. We'll see how
annoying this gets however. Whether we move to a different mechanism,
or just add some interesting higher-level APIs. For example, we may
find a pattern of just asking for 3, so we'll add a simple little
cover utility function to return those (for readability/clarity).
Cheers,
-g
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-09-09 18:17:06 CEST