[WIP] Fix rendering of first_line == None in diff comments
Review Request #10352 — Created Dec. 6, 2018 and updated
Fixing rendering of None first_line
Description | From | Last Updated |
---|---|---|
It seems there's many ways that we could go about this change, potential issues: For the Django diff comment rendering … |
alextechcc |
-
-
It seems there's many ways that we could go about this change, potential issues:
For the Django diff comment rendering side, we have a few options:
1. Make a new template instead ofreviewboard/templates/reviews/diff_comment_fragment.html
, with diff rendering stuff removed
2. Modify template to deal with None's / weird context passed to it
3. Modify the context passed to the template so that it renders in a way that makes sense. (disable controls, 0 lines above, 0 lines below) This is how the current patch does it.There's decent chunks of code, in:
-reviewboard/static/rb/js/resources/models/diffCommentModel.es6.js
-reviewboard/static/rb/js/resources/models/tests/diffCommentModelTests.js
That relies on positive, valid line numbers / number of lines.
I'm also not sure if there's any code I'm missing here that touches / relies on line numbers.