git hyper-blame: Added approx. line number translation.

git hyper-blame: Added approx. line number translation.

Previously, when a commit was skipped, it would be blamed on the line
number the line had *after* the skipped commit. This could mean a
totally unrelated commit gets blamed. Now, a heuristic analyses the diff
of the skipped commit to discover approximately what line number the
line had *before* the skipped commit, so it can hopefully be blamed on
the right commit.

BUG=574290
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=298609

[Matt Giuca, 2016-01-28 08:02:52]

Patchset #4 (id:60001) has been deleted

[Matt Giuca, 2016-02-04 04:48:18]

mgiuca@chromium.org changed reviewers:
+ iannucci@chromium.org

[Matt Giuca, 2016-02-04 04:48:18]

This patch changes git hyper-blame from "sort of OK" to "pretty great" at
identifying which commit an ignored line belongs to (or rather, which line
number that line was on at the old commit, which you can then blame on a
commit).

The key improvement is shown in
GitHyperBlameLineMotionTest.testInterHunkLineMotion: Line 4 is now correctly
blamed on A, and Line 6 is now correctly blamed on B.

[iannucci, 2016-02-05 00:55:55]

nice! lgtm :)

https://codereview.chromium.org/1629253002/diff/120001/git_common.py
File git_common.py (right):

https://codereview.chromium.org/1629253002/diff/120001/git_common.py#newcode378
git_common.py:378: return run('diff', oldrev, newrev, *args)
not sure this helper is /entirely/ warranted since it's 1 line and only used in
one place :D

https://codereview.chromium.org/1629253002/diff/120001/man/src/git-hyper-blam...
File man/src/git-hyper-blame.txt (left):

https://codereview.chromium.org/1629253002/diff/120001/man/src/git-hyper-blam...
man/src/git-hyper-blame.txt:60: blamed.
maybe mention the new caveat about trying to heuristically determine the new
line, but large intra-line changes can foil this?

[Matt Giuca, 2016-02-05 02:46:33]

https://codereview.chromium.org/1629253002/diff/120001/git_common.py
File git_common.py (right):

https://codereview.chromium.org/1629253002/diff/120001/git_common.py#newcode378
git_common.py:378: return run('diff', oldrev, newrev, *args)
On 2016/02/05 00:55:55, iannucci wrote:
> not sure this helper is /entirely/ warranted since it's 1 line and only used
in
> one place :D

Hmm OK, I wasn't sure of the convention. I assumed any git command used should
have a helper so there is a consistent style for git commands (calling dedicated
functions rather than run()).

But I see lots of direct calls to diff elsewhere. Since diff is actually fairly
well used, let's keep this here and maybe migrate more over in the future.

https://codereview.chromium.org/1629253002/diff/120001/man/src/git-hyper-blam...
File man/src/git-hyper-blame.txt (left):

https://codereview.chromium.org/1629253002/diff/120001/man/src/git-hyper-blam...
man/src/git-hyper-blame.txt:60: blamed.
On 2016/02/05 00:55:55, iannucci wrote:
> maybe mention the new caveat about trying to heuristically determine the new
> line, but large intra-line changes can foil this?

Done.

[Matt Giuca, 2016-02-05 03:23:30]

The CQ bit was checked by mgiuca@chromium.org

[Matt Giuca, 2016-02-05 03:23:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
iannucci@chromium.org
Link to the patchset: https://codereview.chromium.org/1629253002/#ps140001
(title: "Added Caveats section to man page.")

[commit-bot: I haz the power, 2016-02-05 03:23:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1629253002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1629253002/140001

[commit-bot: I haz the power, 2016-02-05 03:25:42]

Description was changed from

==========
git hyper-blame: Added approx. line number translation.

Previously, when a commit was skipped, it would be blamed on the line
number the line had *after* the skipped commit. This could mean a
totally unrelated commit gets blamed. Now, a heuristic analyses the diff
of the skipped commit to discover approximately what line number the
line had *before* the skipped commit, so it can hopefully be blamed on
the right commit.

BUG=574290
==========

to

==========
git hyper-blame: Added approx. line number translation.

Previously, when a commit was skipped, it would be blamed on the line
number the line had *after* the skipped commit. This could mean a
totally unrelated commit gets blamed. Now, a heuristic analyses the diff
of the skipped commit to discover approximately what line number the
line had *before* the skipped commit, so it can hopefully be blamed on
the right commit.

BUG=574290

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=298609
==========

[commit-bot: I haz the power, 2016-02-05 03:25:42]

Committed patchset #7 (id:140001) as
http://src.chromium.org/viewvc/chrome?view=rev&revision=298609