> 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.
How about "--source-prop-encoding"?
>> > 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.
Okay.
Note that I will be reverting the renames for now, but the variables
will have the same name in a different patch to rename them.
> 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.
I agree.
>> - 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.
Okay.
Received on 2011-01-07 22:09:59 CET