Git blame for better diffs

When reviewing a pull request against nameko today there was a chunk of code moved from a function into a method, though oterwise mostly unchanged. Unfortunately, it also moved around in the file, and so even git diff -w (abbreviation for git diff --ignore-all-space), ignoring all whitespace showed everything as changed.

Fortunetely, I remembered a trick I found a while ago using git blame:

git blame -s -w -C master..HEAD -- nameko/web/handlers.py |egrep '^[^\^]'

That's blame, -s (suppress author and timestamp in output) -w (ignore whitespace) -C (detect lines moved from other files)

This will show the file, with each line prefixed with information about which commit it came from. However, because we are limiting our diff to the range master..HEAD (from master to the head of the pull request), lines originally written (up to whitespace changes) earlier than master, get marked with ^<sha>, where <sha> is the commit-sha for master. We use egrep to find lines starting with ^.

This can be slightly tricky to read, but there's one more trick:

git blame -s -w -C master..HEAD -- nameko/web/handlers.py |egrep --color=always '^[^\^]|^' |less -R

Here we match either entire lines beginning with ^, or just the beginning of all other lines. That way we can ask egrep to hightligh the matches, and it will highlight just those lines starting with a caret, but output all lines ("highlighting" just the empty start-of-line mark). Finally we pipe to less for pagination, making sure to use -R to re-parse the terminal color codes from egrep.

Original diff

diff --git a/nameko/web/handlers.py b/nameko/web/handlers.py
index a4f6b1a..d9a2c9f 100644
--- a/nameko/web/handlers.py
+++ b/nameko/web/handlers.py
@@ -14,30 +14,6 @@ from nameko.web.server import WebServer
 _log = getLogger(__name__)


-def response_from_result(result):
-    if isinstance(result, Response):
-        return result
-
-    headers = None
-    if isinstance(result, tuple):
-        if len(result) == 3:
-            status, headers, payload = result
-        else:
-            status, payload = result
-    else:
-        payload = result
-        status = 200
-
-    if not isinstance(payload, six.string_types):
-        raise TypeError("Payload must be a string. Got `{!r}`".format(payload))
-
-    return Response(
-        payload,
-        status=status,
-        headers=headers,
-    )
-
-
 class HttpRequestHandler(Entrypoint):
     server = WebServer()

@@ -74,9 +50,42 @@ class HttpRequestHandler(Entrypoint):
                 handle_result=partial(self.handle_result, event))
             result = event.wait()

-            response = response_from_result(result)
+            response = self.response_from_result(result)

         except Exception as exc:
+            response = self.response_from_exception(exc)
+        return response
+
+    def handle_result(self, event, worker_ctx, result, exc_info):
+        event.send(result, exc_info)
+        return result, exc_info
+
+    def response_from_result(self, result):
+        if isinstance(result, Response):
+            return result
+
+        headers = None
+        if isinstance(result, tuple):
+            if len(result) == 3:
+                status, headers, payload = result
+            else:
+                status, payload = result
+        else:
+            payload = result
+            status = 200
+
+        if not isinstance(payload, six.string_types):
+            raise TypeError(
+                "Payload must be a string. Got `{!r}`".format(payload)
+            )
+
+        return Response(
+            payload,
+            status=status,
+            headers=headers,
+        )
+
+    def response_from_exception(self, exc):
         if (
             isinstance(exc, self.expected_exceptions) or
             isinstance(exc, BadRequest)
@@ -87,15 +96,9 @@ class HttpRequestHandler(Entrypoint):
         error_dict = serialize(exc)
         payload = u'Error: {exc_type}: {value}\n'.format(**error_dict)

-            response = Response(
+        return Response(
             payload,
             status=status_code,
         )
-        return response
-
-    def handle_result(self, event, worker_ctx, result, exc_info):
-        event.send(result, exc_info)
-        return result, exc_info
-

 http = HttpRequestHandler.decorator

Blame + egrep

273ee975  53)             response = self.response_from_result(result)
273ee975  56)             response = self.response_from_exception(exc)
273ee975  63)     def response_from_result(self, result):
273ee975  78)             raise TypeError(
273ee975  79)                 "Payload must be a string. Got `{!r}`".format(payload)
273ee975  80)             )
273ee975  88)     def response_from_exception(self, exc):
273ee975  99)         return Response(

Blame-diff

^f5d1f2c   1) from logging import getLogger
^f5d1f2c   2) from functools import partial
^f5d1f2c   3)
^f5d1f2c   4) from eventlet.event import Event
^f5d1f2c   5) import six
^f5d1f2c   6) from werkzeug.wrappers import Response
^f5d1f2c   7) from werkzeug.routing import Rule
^f5d1f2c   8)
^f5d1f2c   9) from nameko.exceptions import serialize, BadRequest
^f5d1f2c  10) from nameko.extensions import Entrypoint
^f5d1f2c  11) from nameko.web.server import WebServer
^f5d1f2c  12)
^f5d1f2c  13)
^f5d1f2c  14) _log = getLogger(__name__)
^f5d1f2c  15)
^f5d1f2c  16)
^f5d1f2c  17) class HttpRequestHandler(Entrypoint):
^f5d1f2c  18)     server = WebServer()
^f5d1f2c  19)
^f5d1f2c  20)     def __init__(self, method, url, expected_exceptions=()):
^f5d1f2c  21)         self.method = method
^f5d1f2c  22)         self.url = url
^f5d1f2c  23)         self.expected_exceptions = expected_exceptions
^f5d1f2c  24)
^f5d1f2c  25)     def get_url_rule(self):
^f5d1f2c  26)         return Rule(self.url, methods=[self.method])
^f5d1f2c  27)
^f5d1f2c  28)     def setup(self):
^f5d1f2c  29)         self.server.register_provider(self)
^f5d1f2c  30)
^f5d1f2c  31)     def stop(self):
^f5d1f2c  32)         self.server.unregister_provider(self)
^f5d1f2c  33)         super(HttpRequestHandler, self).stop()
^f5d1f2c  34)
^f5d1f2c  35)     def get_entrypoint_parameters(self, request):
^f5d1f2c  36)         args = (request,)
^f5d1f2c  37)         kwargs = request.path_values
^f5d1f2c  38)         return args, kwargs
^f5d1f2c  39)
^f5d1f2c  40)     def handle_request(self, request):
^f5d1f2c  41)         request.shallow = False
^f5d1f2c  42)         try:
^f5d1f2c  43)             context_data = self.server.context_data_from_headers(request)
^f5d1f2c  44)             args, kwargs = self.get_entrypoint_parameters(request)
^f5d1f2c  45)
^f5d1f2c  46)             self.check_signature(args, kwargs)
^f5d1f2c  47)             event = Event()
^f5d1f2c  48)             self.container.spawn_worker(
^f5d1f2c  49)                 self, args, kwargs, context_data=context_data,
^f5d1f2c  50)                 handle_result=partial(self.handle_result, event))
^f5d1f2c  51)             result = event.wait()
^f5d1f2c  52)
273ee975  53)             response = self.response_from_result(result)
^f5d1f2c  54)
^f5d1f2c  55)         except Exception as exc:
273ee975  56)             response = self.response_from_exception(exc)
^f5d1f2c  57)         return response
^f5d1f2c  58)
^f5d1f2c  59)     def handle_result(self, event, worker_ctx, result, exc_info):
^f5d1f2c  60)         event.send(result, exc_info)
^f5d1f2c  61)         return result, exc_info
^f5d1f2c  62)
273ee975  63)     def response_from_result(self, result):
^f5d1f2c  64)         if isinstance(result, Response):
^f5d1f2c  65)             return result
^f5d1f2c  66)
^f5d1f2c  67)         headers = None
^f5d1f2c  68)         if isinstance(result, tuple):
^f5d1f2c  69)             if len(result) == 3:
^f5d1f2c  70)                 status, headers, payload = result
^f5d1f2c  71)             else:
^f5d1f2c  72)                 status, payload = result
^f5d1f2c  73)         else:
^f5d1f2c  74)             payload = result
^f5d1f2c  75)             status = 200
^f5d1f2c  76)
^f5d1f2c  77)         if not isinstance(payload, six.string_types):
273ee975  78)             raise TypeError(
273ee975  79)                 "Payload must be a string. Got `{!r}`".format(payload)
273ee975  80)             )
^f5d1f2c  81)
^f5d1f2c  82)         return Response(
^f5d1f2c  83)             payload,
^f5d1f2c  84)             status=status,
^f5d1f2c  85)             headers=headers,
^f5d1f2c  86)         )
^f5d1f2c  87)
273ee975  88)     def response_from_exception(self, exc):
^f5d1f2c  89)         if (
^f5d1f2c  90)             isinstance(exc, self.expected_exceptions) or
^f5d1f2c  91)             isinstance(exc, BadRequest)
^f5d1f2c  92)         ):
^f5d1f2c  93)             status_code = 400
^f5d1f2c  94)         else:
^f5d1f2c  95)             status_code = 500
^f5d1f2c  96)         error_dict = serialize(exc)
^f5d1f2c  97)         payload = u'Error: {exc_type}: {value}\n'.format(**error_dict)
^f5d1f2c  98)
273ee975  99)         return Response(
^f5d1f2c 100)             payload,
^f5d1f2c 101)             status=status_code,
^f5d1f2c 102)         )
^f5d1f2c 103)
^f5d1f2c 104) http = HttpRequestHandler.decorator

Comments !