Update the types for diff chunks and lines.

 






 

 


  Review Request #14399
  —
  Created April 14, 2025 and updated 

 




















 

  

   Information

  





 
 

  david
 









 
 

  Review Board
 









 
 

  master
 









 
 

  
 









 
 

  
 






 




 

  

   Reviewers

  





 
 

  reviewboard
 









 
 

  
 






 








 





  


 
 

  
This change defines a TypedDict for diff chunks, and a NamedTuple

for diff lines, with definitions and documentation for each element.


Before, these were just an opaque dictionary, and a heterogenous list.

The diff lines were particularly annoying because they were variable

length (if there was no metadata, that element was just left off the

end), and types were annoying (for "real" line numbers, we'd have an

int if the line was present on that side of the diff, but was set to

an empty string if it wasn't). This change technically breaks

compatibility in those two cases. We now always include the metadata

field, set to None if there's no metadata for the line. In the case of

real line numbers which are not present on one side of the diff (for

example, the line number of an added line for the left-hand side of the

diff), we set the data for that to None.


The diff_lines template tag and other consumers now use names instead

of indexing magically into the structure, making it much clearer what's

going on.


This also updates the regions field to always be a list. In some cases

we had empty lists, and in others we had None.


This change only touches code related to chunks and lines. Other diff

viewer cleanup is coming in later changes.

 








  


 
 

  
    

  • Tested a ton of different diffs, both in the diff viewer as well as
    
  the text and markdown Review UIs. Verified that all types of edits
    
  (added/edited/deleted/moved) lines all worked as expected.
    • 

  • Ran unit tests.
    • 


 







 









 





  

 


 
 

  



 

  
   


    Summary
    ID

   

  
  

   


    

     

      Update the types for diff chunks and lines.
      
This change defines a `TypedDict` for diff chunks, and a `NamedTuple`
for diff lines, with definitions and documentation for each element.

Before, these were just an opaque dictionary, and a heterogenous list.
The diff lines were particularly annoying because they were variable
length (if there was no metadata, that element was just left off the
end), and types were annoying (for "real" line numbers, we'd have an
int if the line was present on that side of the diff, but was set to
an empty string if it wasn't). This change technically breaks
compatibility in those two cases. We now always include the metadata
field, set to `None` if there's no metadata for the line. In the case of
real line numbers which are not present on one side of the diff (for
example, the line number of an added line for the left-hand side of the
diff), we set the data for that to `None`.

The `diff_lines` template tag and other consumers now use names instead
of indexing magically into the structure, making it much clearer what's
going on.

This also updates the regions field to always be a list. In some cases
we had empty lists, and in others we had `None`.

This change only touches code related to chunks and lines. Other diff
viewer cleanup is coming in later changes.

Testing Done:
- Tested a ton of different diffs, both in the diff viewer as well as
  the text and markdown Review UIs. Verified that all types of edits
  (added/edited/deleted/moved) lines all worked as expected.
- Ran unit tests.

     


    		
    43f82cea94288ecb0ca769be4b38c080b27fd028

   



  
 




 













 






 
 
 

  

   

    

     
    

    
    
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
    

   

   

    
     

      Description
      From
      Last Updated
     

    
    



     

      
       
       
closing bracket does not match indentation of opening bracket's line Column: 17 Error code: E123

      		
      
       reviewbotreviewbot
      
      
       
      
     




     

      
       
       
'reviewboard.diffviewer.chunk_generator.DiffLine' imported but unused Column: 5 Error code: F401

      		
      
       reviewbotreviewbot
      
      
       
      
     




     

      
       
       
too many blank lines (2) Column: 5 Error code: E303

      		
      
       reviewbotreviewbot
      
      
       
      
     




     

      
       
       
undefined name 'none' Column: 42 Error code: F821

      		
      
       reviewbotreviewbot
      
      
       
      
     


    
   

  

 







 






  

 



















 




 

  


   

   

    
 
Checks run (1 failed, 1 succeeded)


    
   


  

  

  





 

 flake8
 failed.
 





 

 JSHint
 passed.
 













 
flake8

 


 










  

 





 














 



 

  

   




david




   

   

  

 



 















 



 

  

   




david




   

   

  

 



 















 



 

  

   




david




   

   

  

 



 















 



 

  

   




david




   

   

  

 



 















 



 

  

   




david




   

   

  

 



 















 



 

  

   




david




   

   

  

 



 















 



 

  

   




david




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  















  

   

    Description:
   

   

    
     
This change defines a TypedDict for diff chunks, and a NamedTuple
     for diff lines, with definitions and documentation for each element.
     

     
Before, these were just an opaque dictionary, and a heterogenous list.
     The diff lines were particularly annoying because they were variable
     length (if there was no metadata, that element was just left off the
     end), and types were annoying (for "real" line numbers, we'd have an
     int if the line was present on that side of the diff, but was set to
     an empty string if it wasn't). This change technically breaks
     compatibility in those two cases. We now always include the metadata
     field, set to None if there's no metadata for the line. In the case of
     real line numbers which are not present on one side of the diff (for
     example, the line number of an added line for the left-hand side of the
     diff), we set the data for that to None.
     

     
The diff_lines template tag and other consumers now use names instead
     of indexing magically into the structure, making it much clearer what's
     going on.
     

   + 
This also updates the regions field to always be a list. In some cases
   + we had empty lists, and in others we had None.
   + 

     
This change only touches code related to chunks and lines. Other diff
     viewer cleanup is coming in later changes.