Addressing TODO for faster algorithm to convert from file line numbers to filediff line numbers

Review Request #3980 — Created March 20, 2013 and discarded

Information

ReviewBot

Reviewers

Added a function _locate_line() that does a binary search for the chunk that contains the line to be searched and then follows up with another binary search within the chunk to get to the line itself. The function returns the chunk and the row which contains the line number.
Tested by running ReviewBot (cpplint, pep8 -- on 1 Cpp file and 1 python script) with the changes and the comments were updated against the correct line numbers in the file diff.
Description From Last Updated

This is returning None in some cases where it shouldn't be.

SM smacleod

Col: 1 W191 indentation contains tabs

reviewbotreviewbot

Col: 1 E101 indentation contains mixed spaces and tabs

reviewbotreviewbot

Col: 35 E702 multiple statements on one line (semicolon)

reviewbotreviewbot
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        bot/reviewbot/processing/review.py
      Ignored Files:
    
    
  2. 
      
SO
SM
  1. While testing this change I've found it fails to locate some line numbers it should be able to find. I've only seen it happen with unmodified lines, but haven't run nearly enough tests yet. I'm going to dig deeper into the logic here and try and figure out what's happening.
  2. bot/reviewbot/processing/review.py (Diff revision 1)
     
     
    Show all issues
    This is returning None in some cases where it shouldn't be.
    1. Thanks for testing it out Steven. Do you have the test diff with which this failed? I could try to debug it from my end as well.
    2. I've emailed you a diff and a script which can be used for testing. Let me know if you find anything.
    3. Looks like "del(self.diff_data.chunks[mid_chunk])" is not having an effect on the list. As a result the index was getting modified but the list itself was not being updated. We ended up skipping past the chunks that had the line number we were looking for.
      
      Interestingly (to me at least, someone well versed with python may be able to explain what's happening), if I take a reference to the list, and then call del on the mid_chunk the item gets deleted from the list.
      
      I've tested the code with the script Steven shared with me:
      
      sundeep@RBserver:~/Documents$ python rbotprofile.py 
      sundeep@RBserver:~/Documents$ 
      
      No errors reported.
  3. 
      
SO
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        bot/reviewbot/processing/review.py
      Ignored Files:
    
    
  2. bot/reviewbot/processing/review.py (Diff revision 2)
     
     
    Show all issues
    Col: 1
     W191 indentation contains tabs
    
  3. bot/reviewbot/processing/review.py (Diff revision 2)
     
     
    Show all issues
    Col: 1
     E101 indentation contains mixed spaces and tabs
    
  4. bot/reviewbot/processing/review.py (Diff revision 2)
     
     
    Show all issues
    Col: 35
     E702 multiple statements on one line (semicolon)
    
  5. 
      
SO
reviewbot
  1. This is a review from Review Bot.
      Tool: PEP8 Style Checker
      Processed Files:
        bot/reviewbot/processing/review.py
      Ignored Files:
    
    
  2. 
      
SO
Review request changed
Status:
Discarded