Re: extending the blame callback
From: Julian Foad <julianfoad_at_apache.org>
Date: Mon, 14 Jan 2019 12:36:19 +0000
Stefan, thanks for taking account of the feedback and updating the doc string in r1851197.
I took a look and thought to rewrite the part about encoding and line splitting like this:
* Character Encoding and Line Splitting:
I dropped the reference to svn_subst_stream_translated() because it wasn't much use without saying what parameters it is given, and instead I was able to say exactly what happens overall.
Problem 1: Using this blame function on a 16-bit character encoding is still really ugly: the receiver cannot know which byte sequences were stripped out.
We should address this issue properly by passing a "line splitter" function in to svn_client_blame6().
Problem 2: Then I noticed that where this splitting algorithm is used, it is in the second pass over the file data, where we associate each "struct blame" item with a line of text.
It looks like a different algorithm may be used in the first pass, when calculating the differences and constructing the blame list. It diffs all repository revisions of the file using svn_diff_file_diff_2() which splits lines according to the "ignore_eol_style" option from svn_client_blame6(diff_options.ignore_eol_style). The effect of "ignore_eol_style" is not entirely clear to me; it is used in svn_diff__normalize_buffer(). In addition, if svn_client_blame6() ends up processing a local WC revision of the file, that is processed through svn_client__get_normalized_stream(normalize_eols=(svn:eol-style == native)) before being diffed.
This is horrible. Surely we should use a consistent "line splitter" everywhere.
I would expect this means it is possible for blame output to go wrong, with revision numbers assigned to the wrong lines. I have been unable to construct such a wrong output, so far, after trying for half an hour or so. Possibly there is no code path that produces such a wrong output. But surely this is the sort of issue that leads to problems down the road.
Can we re-write this properly?
-- - JulianReceived on 2019-01-14 13:36:29 CET |
This is an archived mail posted to the Subversion Dev mailing list.
This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.