[Findit] Bug fixing and implemented some feature requests.

tools/findit/blame.py

Index: tools/findit/blame.py
diff --git a/tools/findit/blame.py b/tools/findit/blame.py
index 630697492e0bf6b10cd4d136d716591348581e52..adf43fc4f495d1e20c458240cd29eb0bfdbaaaeb 100644
--- a/tools/findit/blame.py
+++ b/tools/findit/blame.py
@@ -28,7 +28,7 @@ class Blame(object):
   """
 
   def __init__(self, line_content, component_name, stack_frame_index,
-               file_name, line_number, author, revision,
+               file_name, line_number, author, revision, message,
                url, range_start, range_end):
     # Set all the variables from the arguments.
     self.line_content = line_content
@@ -38,6 +38,7 @@ class Blame(object):
     self.line_number = line_number
     self.author = author
     self.revision = revision
+    self.message = message
     self.url = url
     self.range_start = range_start
     self.range_end = range_end
@@ -56,7 +57,8 @@ class BlameList(object):
   def __getitem__(self, index):
     return self.blame_list[index]
 
-  def FindBlame(self, callstack, crash_revision_dict, regression_dict, parsers,
+  def FindBlame(self, callstack, component_to_crash_revision_dict,
+                component_to_regression_dict, parsers,
                 top_n_frames=10):
     """Given a stack within a stacktrace, retrieves blame information.
 
@@ -65,10 +67,10 @@ class BlameList(object):
 
     Args:
       callstack: The list of stack frames.
-      crash_revision_dict: A dictionary that maps component to its crash
-                          revision.
-      regression_dict: A dictionary that maps component to its revision
-                          range.
+      component_to_crash_revision_dict: A dictionary that maps component to its
+                                        crash revision.
+      component_to_regression_dict: A dictionary that maps component to its
+                                    revision range.
       parsers: A list of two parsers, svn_parser and git_parser
       top_n_frames: A number of stack frames to show the blame result for.
     """
@@ -80,10 +82,11 @@ class BlameList(object):
       # If the component this line is from does not have a crash revision,
       # it is not possible to get blame information, so ignore this line.
       component_path = stack_frame.component_path
-      if component_path not in crash_revision_dict:
+      if component_path not in component_to_crash_revision_dict:
         continue
 
-      crash_revision = crash_revision_dict[component_path]['revision']
+      crash_revision = component_to_crash_revision_dict[
+          component_path]['revision']
       range_start = None
       range_end = None
       is_git = utils.IsGitHash(crash_revision)
@@ -95,8 +98,9 @@ class BlameList(object):
       # If the revision is in SVN, and if regression information is available,
       # get it. For Git, we cannot know the ordering between hash numbers.
       if not is_git:
-        if regression_dict and component_path in regression_dict:
-          component_object = regression_dict[component_path]
+        if component_to_regression_dict and \
+            component_path in component_to_regression_dict:
+          component_object = component_to_regression_dict[component_path]
           range_start = int(component_object['old_revision'])
           range_end = int(component_object['new_revision'])
 
@@ -120,20 +124,20 @@ class BlameList(object):
     component_name = stack_frame.component_name
     file_name = stack_frame.file_name
     file_path = stack_frame.file_path
-    crashed_line_number = stack_frame.crashed_line_number
+    crashed_line_number = stack_frame.crashed_line_number[0]

[Martin Barbella, 2014/08/22 02:24:13]

I mention this in more detail in stacktrace.py, but the name crashed_line_number
is confusing here.

[jeun, 2014/08/22 22:58:43]

Done.

 
     # Parse blame information.
     parsed_blame_info = repository_parser.ParseBlameInfo(
         component_path, file_path, crashed_line_number, crash_revision)
 
     # If it fails to retrieve information, do not do anything.
-    if not parsed_blame_info or len(parsed_blame_info) != 4:
+    if not parsed_blame_info or len(parsed_blame_info) != 5:

[Martin Barbella, 2014/08/22 02:24:13]

It doesn't seem like a good practice to allow parsed_blame_info to ever return
something other than None or exactly 5 results. You can remove the length check
altogether so long as this is the case.

[stgao, 2014/08/22 06:50:53]

+1

Why do we use 5 here?

[jeun, 2014/08/22 22:58:43]

Done.

[jeun, 2014/08/22 22:58:43]

Done.

       return
 
     # Create blame object from the parsed info and add it to the list.
-    (line_content, revision, author, url) = parsed_blame_info
+    (line_content, revision, author, url, message) = parsed_blame_info
     blame = Blame(line_content, component_name, stack_frame_index, file_name,
-                  crashed_line_number, author, revision, url,
+                  crashed_line_number, author, revision, message, url,
                   range_start, range_end)
 
     with self.blame_list_lock: