Store insert/deleted line counts along with diffs.

 






 

 


  Review Request #3816
  —
  Created Jan. 30, 2013 and submitted 

 


















 

  

   Information

  





 
 

  chipx86
 









 
 

  Review Board
 









 
 

  master
 









 
 

  
 









 
 

  
 






 




 

  

   Reviewers

  





 
 

  reviewboard
 









 
 

  
 






 








 





  


 
 

  
Store insert/deleted line counts along with diffs.

When parsing a diff, we now keep track of the inserted/deleted line
counts and store them in the resulting file info. This is then stored in
the FileDiffData for a diff blob. We'll be able to use this later to
display a nice visualization of how many lines were added/removed.

When accessing line counts for older diffs, the diff will be migrated to
a FileDiffData (if needed) and line counts will be recomputed by
re-parsing the stored diff and pulling out those counts.

Unit tests were added to test parsing these for standard diffs and git
diffs (which does a bunch of custom parsing).

 








  


 
 

  
All unit tests pass.

I uploaded some diffs and checked that the correct values were stored
in the FileDiffData. I made sure the recalculate code wasn't being called
for these tests.

I then loaded up some older diffs (some in the diff viewer first, some just
in the admin UI) and saw the correct counts. I checked that the recalculate
code was being called here (and migration in some cases).

 







 









 





  








 









 






  

 








 














 





 

  

   




reviewbot




   

   





  

 



 

  


   

   

    
 
reviewbot


    
   


  

  

  



    




  1. 
 
    
  
    
   
    This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    reviewboard/diffviewer/admin.py
    reviewboard/diffviewer/parser.py
    reviewboard/scmtools/git.py
    reviewboard/diffviewer/models.py
    reviewboard/scmtools/tests.py
    reviewboard/diffviewer/tests.py
    reviewboard/diffviewer/evolutions/__init__.py
    reviewboard/diffviewer/forms.py
  Ignored Files:


    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




chipx86




   

   

  

 



 















 





 

  

   




reviewbot




   

   





  

 



 

  


   

   

    
 
reviewbot


    
   


  

  

  



    




  1. 
 
    
  
    
   
    This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    reviewboard/diffviewer/admin.py
    reviewboard/diffviewer/parser.py
    reviewboard/scmtools/git.py
    reviewboard/diffviewer/evolutions/filediffdata_line_counts.py
    reviewboard/diffviewer/models.py
    reviewboard/scmtools/tests.py
    reviewboard/diffviewer/tests.py
    reviewboard/diffviewer/evolutions/__init__.py
    reviewboard/diffviewer/forms.py
  Ignored Files:


    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   




david




   

   





  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    It looks like the parent branch wasn't set correctly when you posted this--I see parts of other changes in here.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




chipx86




   

   

  

 



 















 





 

  

   




reviewbot




   

   





  

 



 

  


   

   

    
 
reviewbot


    
   


  

  

  



    




  1. 
 
    
  
    
   
    This is a review from Review Bot.
  Tool: PEP8 Style Checker
  Processed Files:
    reviewboard/diffviewer/admin.py
    reviewboard/diffviewer/parser.py
    reviewboard/scmtools/git.py
    reviewboard/diffviewer/evolutions/filediffdata_line_counts.py
    reviewboard/diffviewer/models.py
    reviewboard/scmtools/tests.py
    reviewboard/diffviewer/tests.py
    reviewboard/diffviewer/evolutions/__init__.py
    reviewboard/diffviewer/forms.py
  Ignored Files:


    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   




david




   

   





  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/scmtools/git.py
       


        (Diff revision 3)


       
      
     
    
        
    
     
     
    

     
     
    

     
     
    

     
     
    

     
     
    

     
     
    

     
     
    

     
     
    

     
     
    

     
     
    

     
     
    

     
     
    

     
     
    

     
     
    

     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    I'm not sure how this is related?
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      Git does a bunch of custom diff parsing, so it doesn't end up using the code in the base DiffParser for parsing the idff lines. It needed to be modified to call parse_diff_line to factor in the - and + lines.

The old logic needed updating to parse the --- and +++ lines (as determined by is_diff_fromfile_line) without going through parse_diff_line. To make it a little more clear what the code is doing, I've converted all these to elif/else statements instead of standalone ifs, so that we're saying "if it's not any of these, it's a standard line of diff data, so parse it and factor in -/+".
      
 
      

      2. 


 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 





 

  

   




david




   

   







  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Sounds good. Time for the "deleted a bunch of code" trophy?
    
  
    
 
    

 
    
  



 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




chipx86




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  







  

   

    Status:
   

   


    Completed

   

  




  

   

    Change Summary:
   

   

    
Pushed to master (e8378bfd691f6016807b83e24ae4a3d6fbf6b02f)