Give FileDiffs control over handling of encodings for source files.
Review Request #10775 — Created Nov. 1, 2019 and updated
Historically, Review Board has depended on source files to either us a
UTF-8 or ISO-8859-15 encoding, or for the repository to supply a list of
compatible encodings. If a special encoding were ever used for a file,
and the repository later had its encodings list changed to exclude it,
file would no longer be able to be viewed in the diff viewer.
This encoding list was computed up-front when generating a diff for
file. We'd fetch the repository encoding list, pass it to calls like
get_original_file(), which would then pass that on to
convert_to_unicode(), which would try the built-in encodings and the
provided list of encodings. The caller was always responsible for
fetching or computing that encoding list up-front before calling any of
This change rewrites all of this to make the process more dependent on
FileDiffwe're working on, and to record a known working encoding
for future use.
FileDiff.extra_datacan now contain an
FileDiff.encoding), which specifies an explicit encoding to
use. This could in theory be set during diff parsing (which requires
some additional work still), but it will be set after we've successfully
converted a source file to Unicode and back.
If not set, we still respect the provided encoding list (though this is
now deprecated in
get_original_file()), and then look up from the
repository. This will allow us to later work with
multiple repositories without placing unnecessary burdens on the caller.
The machinery for diff chunk generation has been updated as well. Now,
RawDiffChunkGenerator.generate_chunks()can be given an encoding list
for the old and new content. This defaults to the encoding list normally
provided to the chunk generator, but subclasses (
can pass in separate encoding lists, allowing for two
FileDiffs on an
interdiff to be in separate encodings without breaking chunk generation.
Note that it's still possible for a patch to contain content in another
encoding that doesn't match what's in
FileDiff.encoding. The chunk
generator will still try to do the right thing in this case, falling
back to the repository encodings.
Unit tests passed.
Made sure my various test diffs and interdiffs were still working as
Fixed a name conflict for a test function, and a broken check it masked.
Revision 2 (+733 -94)
Checks run (2 succeeded)
- Based this on top of /r/10776/, which implements
FileDiff.get_repository()(instead of including it in this change)
- Updated a couple more calls to
Revision 3 (+731 -104)