Re: svn commit: r1544182 - in /subversion/trunk/subversion: include/svn_client.h libsvn_client/cat.c libsvn_client/deprecated.c

Bert Huijben wrote:
>> Shouldn't we have an EOL-style translation option as well? Usually the
>> two go  together.
>
> Not sure.
>
> svn_client_cat2() already implemented keyword expansion, but I don't think
> it changes EOL?
>
>
> I just added an option to allow disabling a magic feature of
> svn_client_cat2() that api developers could only work around by directly
> going to either the wc or ra apis.
>
> Translating the EOL style can be done by wrapping the passed output stream
> by a translation... So I don't see that as an as-interesting case.

OK.

>>> + * @param[in] expand_keywords  When true, keywords (when set) are
>>>  expanded.
>>
>> And what if @a expand_keywords is false -- are keywords contracted to their
>> repository-normal form, or left unchanged? The implementation looks like it
>> leaves them unchanged.
>
> We should never have expanded keywords in the repository normal form...
>
> And we certainly never retract them when translating from that form...

Yes, but 'cat' can also operate on a WC path with revision = 'working', and in that case it's not clear whether the caller should expect keywords to be contracted or left as-is.

> I removed the documentation for pool and the todo in r1581796.

Thanks for that and the other tweaks.

>>> +/**
>>> + * Similar to svn_client_cat3() except without the option of directly
>>> + * reading the properties, and with @a expand_keywords always TRUE.
>>
>> ... and only one pool.
>
> I don't think we document those pool changes in other similar cases.
> (svn_client_cat2() didn't have output arguments, so I don't think there
> is anything special to document here)

We do normally document the pools in public API functions, although I find it often seems rather redundant. (In some cases there can be non-obvious implications but we aren't always careful to document those...)

- Julian
Received on 2014-03-26 13:55:50 CET