Fix several GitHub code linking problems and add unit tests.

 






 

 


  Review Request #12532
  —
  Created Aug. 14, 2022 and submitted 

 




















 

  

   Information

  





 
 

  chipx86
 









 
 

  beanbag-docutils
 









 
 

  master
 









 
 

  
 









 
 

  
 






 




 

  

   Reviewers

  





 
 

  beanbag-misc
 









 
 

  
 






 








 





  


 
 

  
We had a Python 3 incompatibility issue where a byte string was ending

up in the format string for the final URL when linking to code on

GitHub, resulting in bad links. The source of the problem turned out to

be in _get_git_doc_ref(), which accepted the resulting byte string

from _run_git() and passed it along.


This issue was first found by Simon Willison, who helpfully provided a

pull request (https://github.com/beanbaginc/beanbag-docutils/pull/2) for

this problem.


The lack of test coverage caused us to miss this issue. In the

development of new tests, additional problems were found. Due to the way

we attempted to generate links to code, we failed to link properly to

variables, and could end up linking to the wrong page if a variable was

assigned to an object, class, or function reference.


The reason is that we were walking an object path (a.b.c.d), and

assuming that the last item (d) was something that resided owned by

the module. Instead, for variables, fetching d really just fetched the

value of the variable. That caused the resulting link to point to the

wrong place entirely, and that place could be a 404, due to how links

are generated.


The better solution is to avoid the inspect machinery we were using,

which can't deal with this situation, and instead parse the AST of the

file. Through this, we can reliably look up the identifier belonging to

the module and get the resulting line number.


This change incorporates that along with the fix for byte strings,

bringing Python 3.6-3.11 compatibility for this module (we will be

dropping 2.7 from beanbag-docutils 2.0).

 








  


 
 

  
All unit tests pass.


Built the Review Board docs, and verified that all the broken links I

could reproduce (including ones pointing to the wrong places) were now

correctly pointing to the correct files and line numbers on GitHub.

 







 









 





  

 


 
 

  



 

  
   


    Summary
    ID

   

  
  

   


    

     

      Fix several GitHub code linking problems and add unit tests.
      
We had a Python 3 incompatibility issue where a byte string was ending
up in the format string for the final URL when linking to code on
GitHub, resulting in bad links. The source of the problem turned out to
be in `_get_git_doc_ref()`, which accepted the resulting byte string
from `_run_git()` and passed it along.

This issue was first found by Simon Willison, who helpfully provided a
pull request (https://github.com/beanbaginc/beanbag-docutils/pull/2) for
this problem.

The lack of test coverage caused us to miss this issue. In the
development of new tests, additional problems were found. Due to the way
we attempted to generate links to code, we failed to link properly to
variables, and could end up linking to the wrong page if a variable was
assigned to an object, class, or function reference.

The reason is that we were walking an object path (`a.b.c.d`), and
assuming that the last item (`d`) was something that resided owned by
the module. Instead, for variables, fetching `d` really just fetched the
value of the variable. That caused the resulting link to point to the
wrong place entirely, and that place could be a 404, due to how links
are generated.

The better solution is to avoid the `inspect` machinery we were using,
which can't deal with this situation, and instead parse the AST of the
file. Through this, we can reliably look up the identifier belonging to
the module and get the resulting line number.

This change incorporates that along with the fix for byte strings,
bringing Python 3.6-3.11 compatibility for this module (we will be
dropping 2.7 from beanbag-docutils 2.0).

     


    		
    8d42a37256e2d9f6ad28517cc1e9f6f915fbc201

   



  
 




 













 






 
 
 

  

   

    

     
    

    
    
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
     
  • 
      
     
    • 
    

   

   

    
     

      Description
      From
      Last Updated
     

    
    



     

      
       
       
Neat - learned about https://pypi.org/project/kgb/ from this! Thanks.

      		
      
       simonwsimonw
      
      
       
      
     




     

      
       
       
undefined name 'e' Column: 38 Error code: F821

      		
      
       reviewbotreviewbot
      
      
       
      
     




     

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

      		
      
       reviewbotreviewbot
      
      
       
      
     




     

      
       
       
expected 2 blank lines after class or function definition, found 1 Column: 1 Error code: E305

      		
      
       reviewbotreviewbot
      
      
       
      
     


    
   

  

 







 






  

 



















 




 

  


   

   

    
 
Checks run (1 failed, 1 succeeded)


    
   


  

  

  





 

 flake8
 failed.
 





 

 JSHint
 passed.
 













 
flake8

 


 










  

 





 














 



 

  

   




chipx86




   

   

  

 



 















 



 

  

   




chipx86




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  









  

   

    Change Summary:
   

   

    
Added missing unit test coverage.

   

  








  

   

    Commits:
   

   

    





 

  
   


    

    Summary
    ID

   

  
  

   


    

    

     

      Fix several GitHub code linking problems and add unit tests.
      
We had a Python 3 incompatibility issue where a byte string was ending
up in the format string for the final URL when linking to code on
GitHub, resulting in bad links. The source of the problem turned out to
be in `_get_git_doc_ref()`, which accepted the resulting byte string
from `_run_git()` and passed it along.

This issue was first found by Simon Willison, who helpfully provided a
pull request (https://github.com/beanbaginc/beanbag-docutils/pull/2) for
this problem.

The lack of test coverage caused us to miss this issue. In the
development of new tests, additional problems were found. Due to the way
we attempted to generate links to code, we failed to link properly to
variables, and could end up linking to the wrong page if a variable was
assigned to an object, class, or function reference.

The reason is that we were walking an object path (`a.b.c.d`), and
assuming that the last item (`d`) was something that resided owned by
the module. Instead, for variables, fetching `d` really just fetched the
value of the variable. That caused the resulting link to point to the
wrong place entirely, and that place could be a 404, due to how links
are generated.

The better solution is to avoid the `inspect` machinery we were using,
which can't deal with this situation, and instead parse the AST of the
file. Through this, we can reliably look up the identifier belonging to
the module and get the resulting line number.

This change incorporates that along with the fix for byte strings,
bringing Python 3.6-3.11 compatibility for this module (we will be
dropping 2.7 from beanbag-docutils 2.0).

     


    		
    1e8070fd24eeb2c07a25b4a0f821fa348206311a

   




   


    

    

     

      Fix several GitHub code linking problems and add unit tests.
      
We had a Python 3 incompatibility issue where a byte string was ending
up in the format string for the final URL when linking to code on
GitHub, resulting in bad links. The source of the problem turned out to
be in `_get_git_doc_ref()`, which accepted the resulting byte string
from `_run_git()` and passed it along.

This issue was first found by Simon Willison, who helpfully provided a
pull request (https://github.com/beanbaginc/beanbag-docutils/pull/2) for
this problem.

The lack of test coverage caused us to miss this issue. In the
development of new tests, additional problems were found. Due to the way
we attempted to generate links to code, we failed to link properly to
variables, and could end up linking to the wrong page if a variable was
assigned to an object, class, or function reference.

The reason is that we were walking an object path (`a.b.c.d`), and
assuming that the last item (`d`) was something that resided owned by
the module. Instead, for variables, fetching `d` really just fetched the
value of the variable. That caused the resulting link to point to the
wrong place entirely, and that place could be a 404, due to how links
are generated.

The better solution is to avoid the `inspect` machinery we were using,
which can't deal with this situation, and instead parse the AST of the
file. Through this, we can reliably look up the identifier belonging to
the module and get the resulting line number.

This change incorporates that along with the fix for byte strings,
bringing Python 3.6-3.11 compatibility for this module (we will be
dropping 2.7 from beanbag-docutils 2.0).

     


    		
    8d42a37256e2d9f6ad28517cc1e9f6f915fbc201

   



  
 







   

  





  

   

    Diff:
   

   

    
 Revision 3 (+1354 -36)
Show changes
 
    AUTHORS 
    dev-requirements.txt 
    beanbag_docutils/sphinx/ext/github.py 
    beanbag_docutils/sphinx/ext/tests/test_github.py 
    beanbag_docutils/sphinx/ext/tests/testdata/__init__.py 
    beanbag_docutils/sphinx/ext/tests/testdata/github_linkcode_module.py 
 

   

  











 

  
Checks run (1 failed, 1 succeeded)



  

 flake8
 failed.
 





  

 JSHint
 passed.
 





 







 
flake8

 


 















  

 
















 





 

  

   




david




   

   







  

 



 

  


   

   

    
 
david


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   




simonw




   

   







  

 



 

  


   

   

    
 
simonw


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 







  2. 
 
    
  
  
    
  
   
    
    
    




    
 
    
  
  
  

  
  
   Show all issues
  
 
    

    



    
    Neat - learned about https://pypi.org/project/kgb/ from this! Thanks.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      It's one of my favorite things I've written :) Lots of advantages over mocks, a key thing being that it leaves the objects and function instances intact, so fewer chances of side effects. We use it heavily.
      
 
      

      2. 


 
    


    

 
    

    3. 













  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 



 

  

   




chipx86




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  







  

   

    Status:
   

   


    Completed

   

  




  

   

    Change Summary:
   

   

    
Pushed to master (3e80042)