Ev2 compat.c suggestions for copyfrom url/rev

Suggestions for clarifying copyfrom URL/rev names and usage.

[[[
Index: subversion/libsvn_delta/compat.c
===================================================================
--- subversion/libsvn_delta/compat.c    (revision 1326951)
+++ subversion/libsvn_delta/compat.c    (working copy)
@@ -1152,9 +1152,14 @@
   const char *path;
   svn_kind_t kind;  /* to copy, mkdir, put or set revprops */
   svn_revnum_t base_revision;       /* When committing, the base revision */
-  svn_revnum_t copyfrom_revision;      /* to copy, valid for add and replace */
   svn_checksum_t *new_checksum;   /* An MD5 hash of the new contents, if any */
-  const char *copyfrom_url;       /* to copy, valid for add and replace */
+
+  /* copy-from location, valid for add and replace, else INVALID/NULL */
+  svn_revnum_t copyfrom_revision;
+  /* If (eb->repos_root), is a ### relpath? fspath?, else is a URL.
+   * ### Cleaner to store it always in the same form. */
+  const char *copyfrom_url;
+
   const char *src_file;  /* for put, the source file for contents */
   apr_hash_t *children;  /* const char *path -> struct operation * */
   apr_hash_t *prop_mods; /* const char *prop_name ->
@@ -1239,8 +1244,9 @@
       enum action_code_t action,
       const char *relpath,
       svn_kind_t kind,
-      const char *url,
-      svn_revnum_t rev,
+      const char *url,  /* ### rename to copyfrom_relpath */
+      svn_revnum_t rev,  /* ### rename to copyfrom_rev_or_del_base_rev ??? */
+      /* or better to separate the two usages of 'rev' into two params? */
       apr_hash_t *props,
       const char *src_file,
       svn_checksum_t *checksum,
@@ -1282,13 +1288,14 @@
         current_props = apr_hash_make(scratch_pool);
       else if (operation->copyfrom_url)
         {
-          const char *copyfrom_relpath;
+          const char *copyfrom_relpath;  /* ### rename to old_copyfrom_relpath */
 
           if (eb->repos_root)
             copyfrom_relpath = svn_uri_skip_ancestor(eb->repos_root,
                                                      operation->copyfrom_url,
                                                      scratch_pool);
           else
+            /* ### Here, ->copyfrom_url is assumed to be a relpath, but ... */
             copyfrom_relpath = operation->copyfrom_url;
 
           SVN_ERR(eb->fetch_props_func(&current_props, eb->fetch_props_baton,
@@ -1362,6 +1369,7 @@
                                                               url,
                                                               eb->edit_pool);
       else
+        /* ### ... here, ->copyfrom_url is made to be a fspath. */
         operation->copyfrom_url = apr_pstrcat(eb->edit_pool, "/", url, NULL);
     }
   /* Handle mkdir operations (which can be adds or replacements). */
]]]

- Julian
Received on 2012-04-17 10:38:15 CEST