Update the types for diff chunks and lines.

Review Request #14399 — Created April 14, 2025 and updated — Latest diff uploaded

Information

Review Board
master

Reviewers

This change defines a TypedDict for diff chunks, and a NamedTuple
for diff lines, with definitions and documentation for each element.

Before, these were just an opaque dictionary, and a heterogenous list.
The diff lines were particularly annoying because they were variable
length (if there was no metadata, that element was just left off the
end), and types were annoying (for "real" line numbers, we'd have an
int if the line was present on that side of the diff, but was set to
an empty string if it wasn't). This change technically breaks
compatibility in those two cases. We now always include the metadata
field, set to None if there's no metadata for the line. In the case of
real line numbers which are not present on one side of the diff (for
example, the line number of an added line for the left-hand side of the
diff), we set the data for that to None.

The diff_lines template tag and other consumers now use names instead
of indexing magically into the structure, making it much clearer what's
going on.

This also updates the regions field to always be a list. In some cases
we had empty lists, and in others we had None.

This change only touches code related to chunks and lines. Other diff
viewer cleanup is coming in later changes.

  • Tested a ton of different diffs, both in the diff viewer as well as
    the text and markdown Review UIs. Verified that all types of edits
    (added/edited/deleted/moved) lines all worked as expected.
  • Ran unit tests.

Diff Revision 2

This is not the most recent revision of the diff. The latest diff is revision 7. See what's changed.

orig
1
2
3
4
5
6
7

Commits

First Last Summary ID Author
Update the types for diff chunks and lines.
This change defines a `TypedDict` for diff chunks, and a `NamedTuple` for diff lines, with definitions and documentation for each element. Before, these were just an opaque dictionary, and a heterogenous list. The diff lines were particularly annoying because they were variable length (if there was no metadata, that element was just left off the end), and types were annoying (for "real" line numbers, we'd have an int if the line was present on that side of the diff, but was set to an empty string if it wasn't). This change technically breaks compatibility in those two cases. We now always include the metadata field, set to `None` if there's no metadata for the line. In the case of real line numbers which are not present on one side of the diff (for example, the line number of an added line for the left-hand side of the diff), we set the data for that to `None`. The `diff_lines` template tag and other consumers now use names instead of indexing magically into the structure, making it much clearer what's going on. This change only touches code related to chunks and lines. Other diff viewer cleanup is coming in later changes. Testing Done: - Tested a ton of different diffs, both in the diff viewer as well as the text and markdown Review UIs. Verified that all types of edits (added/edited/deleted/moved) lines all worked as expected. - Ran unit tests.
541e055e3978c23ea3bba8f9ee0b67c26fc4134f David Trowbridge
reviewboard/diffviewer/chunk_generator.py
reviewboard/diffviewer/diffutils.py
reviewboard/diffviewer/templatetags/difftags.py
reviewboard/diffviewer/tests/test_diff_chunk_generator.py
reviewboard/diffviewer/tests/test_raw_diff_chunk_generator.py
reviewboard/reviews/tests/test_markdown_review_ui.py
reviewboard/reviews/tests/test_text_based_review_ui.py
reviewboard/webapi/tests/test_filediff.py
Loading...