Lines starting with '---', '+++' aren't necessarily diff control lines.

 






 

 


  Review Request #2357
  —
  Created May 9, 2011 and submitted

 


















 

  

   Information

  





 
 

  jshamacher
 









 
 

  RBTools
 









 
 

  
 









 
 

  
 









 
 

  
 






 




 

  

   Reviewers

  





 
 

  rbtools
 









 
 

  
 






 








 





  


 
 

  
We ran into this bug with a file containing a database schema.  It was well commented, so a lot of lines started with '-- ' followed with comment text.  Unfortunately, when these lines were deleted the resulting raw diff contained '--- ', and as the raw diff was processed those lines would hit this code.

 








  


 
 

  
We were able to submit a properly-formed diff of the schema file to ReviewBoard with this patch applied.  Without it, the diff was malformed.

 







 









 





  








 









 






  

 








 














 





 

  

   




chipx86




   

   





  

 



 

  


   

   

    
 
chipx86


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       rbtools/postreview.py
       


        (Diff revision 1)


       
      
     
    
        
    
     
     
    

     
     
    

     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    If this is specific to SVN, it should be stored in the SVNClient class. Also, it should be a bit more clear what it is by the name, and should be a pre-compiled regex.

I'd prefer:

DIFF_ORIG_FILE_LINE_RE = re.compile(r'^---.......')
DIFF_NEW_FILE_LINE_RE = re.compile(r'^\+\+\+.......')
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 






  3. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       rbtools/postreview.py
       


        (Diff revision 1)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    With the regexes compiled, this will be more like: self.DIFF_BLAH.match(line).
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 






  4. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
    
      
       rbtools/postreview.py
       


        (Diff revision 1)


       
      
     
    
        
    
     
     
    

     
     
    

        
   
    
  
    
 
    

   
    
    
    





    
    Wrap to < 80 chars. Use parens for the wrapping, one match per line in this case.
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    5. 




  5. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    6. 




  

 
















 



 

  

   





 JS





   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  









  

   

    Change Summary:
   

   

    
Updated per Christian's feedback.

   

  








  

   

    Diff:
   

   

    
 Revision 2 (+9 -3)
Show changes
 
    rbtools/postreview.py 
 

   

  












  

 
















 





 

  

   




david




   

   







  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Pushed to master as 4c9a57e. Thanks!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3.