the attached patch has some corrections to the doc strings.
If there are no objections, I'll commit this tomorrow.
Also see inline comments:
On 06.01.2019 21:09, Daniel Shahaf wrote:
> Stefan Kueng wrote on Sun, Jan 06, 2019 at 20:40:28 +0100:
>> +++ subversion/include/svn_client.h (working copy)
>> @@ -736,10 +736,11 @@
>> * @{
>> */
>>
>> -/** Callback type used by svn_client_blame5() to notify the caller
>> +/** Callback type used by svn_client_blame6() to notify the caller
>> * that line @a line_no of the blamed file was last changed in @a revision
>> * which has the revision properties @a rev_props, and that the contents were
>> - * @a line.
>> + * @a line. The @a line content is delivered as is, it is up to the client to
>> + * determine the encoding.
>
> s/,/;/
>
> Shouldn't the docstring explicitly say whether or not the value of @a
> line includes the terminating newline? (Preëxisting issue)
Done.
>
>> @@ -758,6 +759,33 @@
>> * will be true if the reason there is no blame information is that the line
>> * was modified locally. In all other cases @a local_change will be false.
>> *
>> + * @note if the text encoding of the file is not ASCII or utf8, the end-of-line
>> + * detection may lead to lines having a one byte offset. It is up to the client
>
> "One byte offset" is not true in general; it is true for UTF-16 but
> there are other encodings in the world. Besides, I would sooner point
> out that if the file isn't in UTF-8 (including ASCII), the end of line
> detection may be *wrong* since it looks for the byte 0x0A, which may not
> even be part of a (possibly multibyte) newline character.
>
> It's fine to give specific details about UTF-16, but we should give the
> more generally-applicable information first.
The wording is "*may*", but I've reworded it slightly. I hope it's better.
>
>> + * to handle these situations. Blaming non ASCII/utf8 files requires the
>
> s/utf8/UTF-8/ (two instances)
Done.
>
>> + * @a force flag to be set when calling the blame function.
>
> s/the blame function/svn_client_blame6/
Done.
>
>> @@ -941,18 +942,20 @@
>> SVN_ERR(ctx->cancel_func(ctx->cancel_baton));
>> if (!eof || sb->len)
>> {
>> + line.data = sb->data;
>> + line.len = sb->len;
>
> Reduce the scope of LINE to just this block?
I think for proper C, all variables need to be declared at the start of
the function. Unless of course svn allows newer C versions?
Received on 2019-01-07 19:30:31 CET