On Wed, Sep 5, 2012 at 3:54 PM, Branko Čibej <br...@wandisco.com> wrote:
>>> As I said, the return value must be a svn_config_t*.
>>> More comments below (I'll ignore coding style).
>> Why must it be an svn_config_t* ?
>> That makes no sense: we want to duplicate the config hash.
>
> Ah, I begin to understand; you want to duplicate the result of
> svn_config_get_config, instead of the result of
> svn_config_create/svn_config_read. I tend to think these are two
> separate functions:
>
> svn_error_t *svn_config_dup(svn_config_t **cfgp, svn_config_t *src,
> apr_pool_t *pool);
> svn_error_t *svn_config_copy_config(apr_hash_t **cfg_hash, apr_hash_t
> *src_hash, apr_pool_t *pool);
>
>
> Sorry about the misunderstanding.
Ok, I'll create two separate dup functions then.
The hash-dup function can just call the config-dup function.
>
>> > And here; svn_config_set already has code to create option
>> > and section records, it only needs to be factored out.
>> Yes, but I don't like the idea of using svn_config_set to do this
>> because it does more than is necessary here.
>> We don't have to check whether to replace or add an entry because we
>> always add (we're copying).
>>
>> And the question is: do we want an exact copy or a new object,
>> initialized with the same data?
>
> I'm not suggesting you use svn_config_set, that would be wrong, because
> it fiddles with value expansions. I'm suggesting that you factor out the
> parts of svn_config_set that create section and options into separate
> static helper functions, so that the new code and svn_config_set can
> both use those. That's better than duplicating the code.
Ok, I'll do some factoring.
Stefan
--
___
oo // \\ "De Chelonian Mobile"
(_,\/ \_/ \ TortoiseSVN
\ \_/_\_/> The coolest Interface to (Sub)Version Control
/_/ \_\ http://tortoisesvn.net
