On Sun, 2011-01-02, Daniel Shahaf wrote: > Danny Trebbien wrote on Sun, Dec 26, 2010 at 15:39:05 -0800: > > Attached are version 3 of the patch and corresponding log message. > > Also attached is a TGZ archive of two Subversion repository dumps that > > are used by a new test case.
> > [[[ > > Add a command line option (--source-encoding) to the svnsync init, sync, and > > copy-revprops subcommands that allows the user to specify the character > > encoding of translatable properties from the source repository. This is > > needed > > to allow svnsync to sync some older Subversion repositories that have > > properties that were not encoded in UTF-8. [...] +1 to what danielsh said. Seems to me it would be good for the name of the command-line option to include "property", e.g. "--prop-source-encoding". I note that this option is not intended to be used frequently so I don't imagine it's a problem to have a long name. > > Index: subversion/svnsync/sync.h > > =================================================================== > > svnsync_normalize_revprops(apr_hash_t *rev_props, > > - int *normalized_count, > > + int *normalized_le_count, > > + const char *encoding, > > apr_pool_t *pool); > > svnsync_get_sync_editor(const svn_delta_editor_t *wrapped_editor, > > void *wrapped_edit_baton, > > svn_revnum_t base_revision, > > const char *to_url, > > + const char *prop_encoding, > > svn_boolean_t quiet, > > const svn_delta_editor_t **editor, > > void **edit_baton, > > - int *normalized_node_props_counter, > > + int *le_normalized_node_props_counter, Can you use the same names for the counter parameters? (I realize one is an output and the other is an in-out, but that doesn't appear to be the reason why they differ.) Also we normally use "EOL" for "end of line"; I haven't noticed the use of "LE" before. Not directly related to this change, but ... > - int *normalized_node_props_counter; /* Where to count normalizations? */ > + int *le_normalized_node_props_counter; /* Where to count line ending > + normalizations? */ > } edit_baton_t; > > @@ -407,9 +425,10 @@ change_file_prop(void *file_baton, > if (svn_prop_needs_translation(name)) > { > svn_boolean_t was_normalized; It would be a good idea to rename this variable also, otherwise this is a bit confusing. > - SVN_ERR(normalize_string(&value, &was_normalized, pool)); > + SVN_ERR(normalize_string(&value, &was_normalized, eb->prop_encoding, > + pool, pool)); > if (was_normalized) > - (*(eb->normalized_node_props_counter))++; > + (*(eb->le_normalized_node_props_counter))++; > } This kind of code can be simplified (removing one level of indirection) by making the baton hold the actual counter instead of a pointer to it. - Julian