• 
      

    Add support for incremental diff expansion.

    Review Request #3327 — Created Sept. 14, 2012 and submitted

    Information

    Review Board
    release-1.6.x

    Reviewers

    Add support for incremental diff expansion.
    
    Review Board has always had the ability to let users expand collapsed
    chunks of diffs, but this can produce a lot of content when dealing with
    large sections of collapsed code.
    
    This feature introduces incremental expansion. Users can expand the
    diffs by 20 lines at a time, before or after the collapsed header. They
    can also expand up to the nearest function listed, and of course, can
    expand te entire header.
    
    When a diff is expanded, even partially, little floating collapse
    buttons will appear near the center column. They'll stay in view,
    centered amongst the visible portion of the expanded region. Clicking
    them will re-collapse that segment.
    
    This feature was sponsored by NetApp, Inc.
    Tested the incremental expansion (pre/post) for chunk headers at the top,
    middle, and bottom of diffs.
    
    Tested that expanding chunks kept the chunk header in place, regardless of
    which side. When fully expanding, the collapse button started where the chunk
    header was.
    
    Tested that fully expanding a chunk continues to work.
    
    Tested that expanding to a class/function works.
    
    Tested that collapsing works in all cases.
    
    Tested that function/class headers update as the chunk is incrementally expanded.
    
    Tested all this with Chrome, Firefox, and IE.
    Description From Last Updated

    The caller that passes this in defaults it to [None, None] too. Can we just have that be the default …

    daviddavid

    Alignment looks off here

    daviddavid

    You can just collapse this into one if statement

    daviddavid

    This first sentence was a little confusing at first. How about this: If we're currently expanding part of a chunk, …

    daviddavid

    Wanna fix this to be !== while you're in here?

    daviddavid

    Alignment?

    daviddavid

    Alignment?

    daviddavid

    Alignment?

    daviddavid
    david
    1. Haven't looked at the code yet, but this also covers issues 603 and 1231, right?
    2. 
        
    david
    1. 
        
    2. reviewboard/diffviewer/diffutils.py (Diff revision 1)
       
       
       
      Show all issues
      The caller that passes this in defaults it to [None, None] too. Can we just have that be the default for the parameter instead of doing None and then having this?
      1. Nope, because in that case, last_header is basically a static variable. We modify it directly, and when you initialize to a list and then modify that list, you'll get those same modifications the next time you call.
    3. reviewboard/diffviewer/diffutils.py (Diff revision 1)
       
       
       
      Show all issues
      Alignment looks off here
      1. Not here. Another instance of the bug you keep hitting I guess.
    4. reviewboard/diffviewer/views.py (Diff revision 1)
       
       
       
       
      Show all issues
      You can just collapse this into one if statement
      1. Can't collapse (the results are the same), but that second shouldn't have checked lines_of_context.
    5. reviewboard/diffviewer/views.py (Diff revision 1)
       
       
       
      Show all issues
      This first sentence was a little confusing at first. How about this:
      
      If we're currently expanding part of a chunk, we want to render the entire chunk without any lines collapsed.
    6. Show all issues
      Wanna fix this to be !== while you're in here?
    7. Show all issues
      Alignment?
      1. I don't see anything wrong.
    8. Show all issues
      Alignment?
    9. Show all issues
      Alignment?
    10. 
        
    chipx86
    david
    1. Ship It!
    2. 
        
    chipx86
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-1.6.x (754e8bf)