Fix blank diff fragments in interdiffs

Review Request #6838 — Created Jan. 29, 2015 and submitted

brennie
Review Board
master
reviewboard

Previously, diff fragment comments from interdiffs were sometimes
resulting in a blank diff fragment. This was because of the diff
fragment viewer relying on the 'total_line_count' value in the
filediff/interfilediff line counts. However, this value is not always
guaranteed to have a value. Now we specifically calculate the last
line in the filediff/interfilediff to avoid this issue.

Ran unit tests.

Verified that https://reviews.reviewboard.org/r/6815/#comment19874
does not appear as a blank diff with the patch applied. To do this I
saved and uploaded all the diffs in order and created an issue on the
same lines in the same interdiff.

  • 0
  • 0
  • 1
  • 0
  • 1
Description From Last Updated
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/diffviewer/diffutils.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/diffviewer/diffutils.py
    
    
  2. 
      
chipx86
  1. Looks good in general. One question about naming.

    More generally, can you structure the review request descriptions to be more high-level, less about the functions (some info there is fine for things like the docstrings). Basically, something along the lines of "Here's what wasn't working. Here's why. Here's how it's better." will help a lot when I go to draft release notes.

  2. reviewboard/reviews/views.py (Diff revision 1)
     
     
     

    Looks like we're going from getting the count to the actual last line? Or is it the last line number? (If the latter, the function should probably be get_last_line_number_in_diff).

  3. 
      
brennie
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/diffviewer/diffutils.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/views.py
        reviewboard/diffviewer/diffutils.py
    
    
  2. 
      
chipx86
  1. Ship It!
  2. 
      
brennie
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to master (300dd5b)
Loading...