Update the types for diff chunks and lines.

 






 

 


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

 


















 

  

   Information

  





 
 

  david
 









 
 

  Review Board
 









 
 

  master
 









 
 

  
 









 
 

  
 






 




 

  

   Reviewers

  





 
 

  reviewboard
 









 
 

  
 






 








 





  


 
 

  
This change defines a TypedDict for diff chunks, and a tuple 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 an empty

string if not). 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. This increases the cached size of a very large

diff with thousands of lines by less that 0.1% (much better than many

years ago with older pickle protocols--the tuple is probably also

helping here).


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. In

order to maintain compatibility with current versions of Review Bot, we

change None line numbers back to an empty string for the filediff

resource's diff_data mode.


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.
    • 


 







 









 





  








 




 







   

    
    
Changes between revision 3 and 4



    

 

 

 

 
orig
1
2
3
4
5
6
7
8
9

 



    


    

      
Commits

      

 

 

  
   
  
  Summary
  ID
  Author
 



 
  
 
 
  
   
    

     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 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.

    

   
  		
  b65d93070af3cf1bc40238f661da52ad4eeacd2d
  David Trowbridge
 


 
  
 
 
  
   
    

     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 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.

    

   
  		
  e2861454c701dc0aa04163b7aa4a1982233c2cb2
  David Trowbridge
 




    


    
    

   

  

 









 

  

   
    

     
      
      reviewboard/diffviewer/chunk_generator.py
     
    

   
  

 










     

    

   

  

  

   
   Loading...