Add function/class headers to collapsed and commented regions of diffs

 






 

 


  Review Request #1300
  —
  Created Dec. 19, 2009 and submitted

 


















 

  

   Information

  





 
 

  chipx86
 









 
 

  Review Board
 









 
 

  master
 









 
 

  114
 









 
 

  
 






 




 

  

   Reviewers

  





 
 

  reviewboard
 









 
 

  
 






 








 





  


 
 

  
This change adds function/class header information to collapsed regions in the diff viewer and to commented regions on reviews.

New functions have been added to the differ classes for registering "interesting line" regexes. These are regular expressions associated with a name that can match lines in a file. Any matched lines are stored in a list associated with that name. Multiple regular expressions can be tied with a name.

A list of regexes for various common file formats are defined that match functions/classes. After performing the diff, we grab the matched lines and store them along with the chunk. We can then use these to show the appropriate function/class in a collapsed region.

Similarly, we can grab out these headers for commented regions of diffs on the review page and in the e-mails.

 








  


 
 

  
Ran a suite of unit tests for each file format we support. They all passed as expected. Tested these with complex Python files and C files and saw the expected results as well.

 







 









 





  







 

  
Screenshots

  
 



 









 






  

 








 














 





 

  

   




david




   

   





  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    I don't see too much that I disagree with here, but it's hard to understand the pitfalls because it's been a long time since I looked at this code.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/diffviewer/myersdiff.py
       


        (Diff revision 1)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    I think you can just say "if stripped_line:" and it'll avoid the O(n) len call.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      I figured the length was stored along with the string contents. I really hope it's not O(n).

Anyways, fixed.
      
 
      

      2. 


 
    


    

 
    

    3. 






  3. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       reviewboard/diffviewer/myersdiff.py
       


        (Diff revision 1)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    You don't seem to use m anywhere else, so you could put the regex.match inline.
    
  
    
 
    

 
    
  



 
    

    4. 




  4. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    5.