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?
[[[
Extend the blame callback with a string length parameter.
* subversion/incluce/svn_client.h
* subversion/libsvn_client/blame.c
(svn_client_blame_receiver4_t): typedef for new callback
(svn_client_blame6): new API using the svn_client_blame_receiver4_t callback
* subversion/libsvn_client/deprecated.c
(svn_client_blame5): moved API there, calling svn_client_blame6 using a
callback shim
(blame_wrapper_receiver3): callback shim for svn_client_blame5
]]]
Index: subversion/include/svn_client.h
===================================================================
--- subversion/include/svn_client.h (revision 1850672)
+++ 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. The line does not contain the cr/lf at the end.
*
* @a start_revnum and @a end_revnum contain the start and end revision
* number of the entire blame operation, as determined from the repository
@@ -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 UTF-8, the
end-of-line
+ * detection may lead to lines not starting at the right offset. It is up to
the
+ * client to handle such situations. Blaming non ASCII/UTF-8 files requires the
+ * @a force flag to be set when calling the svn_client_blame6 function.
+ *
+ * @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);
+
+/**
+ * Similar to #svn_client_blame_receiver4_t, but with the line parameter
+ * as a (const char*) instead of an svn_string_t.
+ *
+ * @deprecated Provided for backward compatibility with the 1.11 API.
+ *
* @since New in 1.7.
*/
typedef svn_error_t *(*svn_client_blame_receiver3_t)(
@@ -2928,6 +2956,28 @@
*
* Use @a pool for any temporary allocation.
*
+ * @since New in 1.12.
+ */
+svn_error_t *
+svn_client_blame6(const char *path_or_url,
+ const svn_opt_revision_t *peg_revision,
+ const svn_opt_revision_t *start,
+ const svn_opt_revision_t *end,
+ const svn_diff_file_options_t *diff_options,
+ svn_boolean_t ignore_mime_type,
+ svn_boolean_t include_merged_revisions,
+ svn_client_blame_receiver4_t receiver,
+ void *receiver_baton,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool);
+
+
+/**
+ * Similar to svn_client_blame6(), but with #svn_client_blame_receiver3_t
+ * as the receiver.
+ *
+ * @deprecated Provided for backwards compatibility with the 1.11 API.
+ *
* @since New in 1.7.
*/
svn_error_t *
@@ -2943,9 +2993,8 @@
svn_client_ctx_t *ctx,
apr_pool_t *pool);
-
/**
- * Similar to svn_client_blame5(), but with #svn_client_blame_receiver3_t
+ * Similar to svn_client_blame5(), but with #svn_client_blame_receiver2_t
* as the receiver.
*
* @deprecated Provided for backwards compatibility with the 1.6 API.
Index: subversion/libsvn_client/blame.c
===================================================================
--- subversion/libsvn_client/blame.c (revision 1850672)
+++ subversion/libsvn_client/blame.c (working copy)
@@ -656,7 +656,7 @@
}
svn_error_t *
-svn_client_blame5(const char *target,
+svn_client_blame6(const char *target,
const svn_opt_revision_t *peg_revision,
const svn_opt_revision_t *start,
const svn_opt_revision_t *end,
@@ -663,7 +663,7 @@
const svn_diff_file_options_t *diff_options,
svn_boolean_t ignore_mime_type,
svn_boolean_t include_merged_revisions,
- svn_client_blame_receiver3_t receiver,
+ svn_client_blame_receiver4_t receiver,
void *receiver_baton,
svn_client_ctx_t *ctx,
apr_pool_t *pool)
@@ -676,6 +676,7 @@
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;
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;
}
Index: subversion/libsvn_client/deprecated.c
===================================================================
--- subversion/libsvn_client/deprecated.c (revision 1850672)
+++ subversion/libsvn_client/deprecated.c (working copy)
@@ -166,7 +166,59 @@
}
/*** From blame.c ***/
+struct blame_receiver_wrapper_baton3 {
+ void *baton;
+ svn_client_blame_receiver3_t receiver;
+};
+static svn_error_t *
+blame_wrapper_receiver3(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)
+{
+ struct blame_receiver_wrapper_baton3 *brwb = baton;
+
+ if (brwb->receiver)
+ return brwb->receiver(brwb->baton, start_revnum, end_revnum, line_no,
+ revision, rev_props, merged_revision,
+ merged_rev_props, merged_path, line->data,
+ local_change, pool);
+
+ return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_client_blame5(const char *target,
+ const svn_opt_revision_t *peg_revision,
+ const svn_opt_revision_t *start,
+ const svn_opt_revision_t *end,
+ const svn_diff_file_options_t *diff_options,
+ svn_boolean_t ignore_mime_type,
+ svn_boolean_t include_merged_revisions,
+ svn_client_blame_receiver3_t receiver,
+ void *receiver_baton,
+ svn_client_ctx_t *ctx,
+ apr_pool_t *pool)
+{
+ struct blame_receiver_wrapper_baton3 baton;
+
+ baton.receiver = receiver;
+ baton.baton = receiver_baton;
+
+ return svn_client_blame6(target, peg_revision, start, end, diff_options,
+ ignore_mime_type, include_merged_revisions,
+ blame_wrapper_receiver3, &baton, ctx, pool);
+}
+
struct blame_receiver_wrapper_baton2 {
void *baton;
svn_client_blame_receiver2_t receiver;
