[WIP] Fix rendering of first_line == None in diff comments

Review Request #10352 — Created Dec. 6, 2018 and updated

alextechcc
Review Board
release-4.0.x
8104379...
reviewboard, students

Fixing rendering of None first_line



Loading file attachments...

  • 1
  • 0
  • 0
  • 0
  • 1
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 alextechcc
Checks run (2 succeeded)
flake8 passed.
JSHint passed.
alextechcc
  1. 
      
  2. 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 of reviewboard/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.

  3. 
      
Loading...