Patrick Steinhardt wrote on Mon, Nov 21, 2016 at 11:57:01 +0100:
> On Sat, Nov 19, 2016 at 05:53:26AM +0000, Daniel Shahaf wrote:
> > Patrick Steinhardt wrote on Wed, Nov 09, 2016 at 12:54:08 +0100:
> > > + if (kind != svn_node_dir)
> > > + return svn_error_createf(
> > > + SVN_ERR_ILLEGAL_TARGET,
> > > + NULL,
> > > + _("Rejecting checkout to existing %s '%s'"),
> > > + svn_node_kind_to_word(kind),
> >
> > Would using 'kind' through %s cause a problem for translators?
> > It would result in a nominative English noun being used in a dative
> > local-language context.
>
> Is there any recommendation what to use instead here?
I think best practice in translations is to repeat the full string:
"Rejecting checkout to existing file '%s'"
"Rejecting checkout to existing directory '%s'"
> > > + svn_pool_clear(scratch_pool);
> > > +
> >
> > This pool usage is not idiomatic.
> >
> > Does this function (verify_checkout_target()) return a value to its
> > caller? If it does, then let its signature have two pools. If it does
> > not, then let it take a single 'apr_pool_t *scratch_pool' argument, use
> > that pool for both pool arguments of callees, and leave that pool for
> > its (verify_checkout_target()'s) caller to clear. (In this case,
> > probably at apr_terminate().)
>
> No, `verify_checkout_target()` does not return a value, it only
> indicates an error if the target is not valid. The reason why I
> created the scratch pool was that I have to pass both a result
> and scratch pool to `svn_client_get_repos_root`, where I wasn't
> exactly sure which semantics one may expect when passing the same
> pool as both result and scratch pool to a callee.
>
> So if I understand it correctly I can expect
> `svn_client_get_repos_root` to behave correct in that case, so I
> can pass a scratch pool twice?
Yes, you can, and it'll work.
The rule is that a 'scratch_pool' argument may be cleared by the
*caller* at any time after the call. Callees don't clear pools passed
to them by callers.
> Thanks for your review.
You're welcome.
Cheers,
Daniel
