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)
> @@ -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.
> + * to handle these situations. Blaming non ASCII/utf8 files requires the
s/utf8/UTF-8/ (two instances)
> + * @a force flag to be set when calling the blame function.
s/the blame function/svn_client_blame6/
> + * @since New in 1.12.
> + */
> +typedef svn_error_t *(*svn_client_blame_receiver4_t)(
> + void *baton,
> + svn_revnum_t start_revnum,
> + svn_revnum_t end_revnum,
> + apr_int64_t line_no,
> + svn_revnum_t revision,
> + apr_hash_t *rev_props,
> + svn_revnum_t merged_revision,
> + apr_hash_t *merged_rev_props,
> + const char *merged_path,
> + const svn_string_t *line,
> + svn_boolean_t local_change,
> + apr_pool_t *pool);
> +++ subversion/libsvn_client/blame.c (working copy)
> @@ -676,6 +676,7 @@ svn_client_blame6(const char *target,
> svn_stream_t *last_stream;
> svn_stream_t *stream;
> const char *target_abspath_or_url;
> + svn_string_t line;
>
> if (start->kind == svn_opt_revision_unspecified
> || end->kind == svn_opt_revision_unspecified)
> @@ -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?
> if (walk->rev)
> SVN_ERR(receiver(receiver_baton, start_revnum, end_revnum,
> line_no, walk->rev->revision,
> walk->rev->rev_props, merged_rev,
> merged_rev_props, merged_path,
> - sb->data, FALSE, iterpool));
> + &line, FALSE, iterpool));
> else
> SVN_ERR(receiver(receiver_baton, start_revnum, end_revnum,
> line_no, SVN_INVALID_REVNUM,
> NULL, SVN_INVALID_REVNUM,
> NULL, NULL,
> - sb->data, TRUE, iterpool));
> + &line, TRUE, iterpool));
> }
> if (eof) break;
> }
Cheers,
Daniel
