On 07.01.2019 19:58, Daniel Shahaf wrote:
Stefan Kueng wrote on Mon, 07 Jan 2019 19:30 +0100:
On 06.01.2019 21:09, Daniel Shahaf wrote:
Stefan Kueng wrote on Sun, Jan 06, 2019 at 20:40:28 +0100:
@@ -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.
It _is_ better, thank you, but I agree with Julian's last post where he wrote
that
the docstring should just say that the line is split on LF bytes. The current
patch's docstring implies the LF byte is necessarily part of a line terminator,
which is true for UTF-8/16/32 but not necessarily true in arbitrary encodings.
next patch attached. I think this is better now.
Stefan
[[[
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 the line is split on LF characters. Clients must be aware of this
+ * when dealing with different encodings of the file/line.
+ * 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)
@@ -941,18 +941,21 @@
SVN_ERR(ctx->cancel_func(ctx->cancel_baton));
if (!eof || sb->len)
{
+ svn_string_t line;
+ 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;
