Merge 500a9ab into 9ca4126

core/commands/diff.py

@@ -3,12 +3,12 @@
 current diff.
 """
 
+from collections import namedtuple
 from contextlib import contextmanager
 from functools import partial
 from itertools import chain, dropwhile, takewhile
 import os
 import re
-import bisect
 
 import sublime
 from sublime_plugin import WindowCommand, TextCommand, EventListener
@@ -20,7 +20,7 @@
 
 
 if False:
-    from typing import Callable, Iterable, Iterator, List, Optional, Tuple, TypeVar
+    from typing import Callable, Iterable, Iterator, List, NamedTuple, Optional, Set, Tuple, TypeVar
     from mypy_extensions import TypedDict
 
     T = TypeVar('T')
@@ -29,11 +29,15 @@
         'hunks': List[Tuple[int, int]]
     })
 
+    Point = int
+    RowCol = Tuple[int, int]
+    HunkLine_ = NamedTuple('HunkLine_', [('mode', str), ('text', str), ('b', int)])
+
 
 DIFF_TITLE = "DIFF: {}"
 DIFF_CACHED_TITLE = "DIFF (cached): {}"
 
-
+HunkLine = namedtuple('HunkLine', 'mode text b')  # type: HunkLine_
 diff_views = {}
 
 
@@ -573,54 +577,178 @@ class GsDiffOpenFileAtHunkCommand(TextCommand, GitCommand):
     """
 
     def run(self, edit):
+        # type: (sublime.Edit) -> None
         # Filter out any cursors that are larger than a single point.
         cursor_pts = tuple(cursor.a for cursor in self.view.sel() if cursor.a == cursor.b)
 
-        diff_starts = tuple(region.a for region in self.view.find_all("^diff"))
-        hunk_starts = tuple(region.a for region in self.view.find_all("^@@"))
-
-        for cursor_pt in cursor_pts:
-            diff_start = diff_starts[bisect.bisect(diff_starts, cursor_pt) - 1]
-            diff_start_line = self.view.substr(self.view.line(diff_start))
-
-            hunk_start = hunk_starts[bisect.bisect(hunk_starts, cursor_pt) - 1]
-            hunk_line_str = self.view.substr(self.view.line(hunk_start))
-            hunk_line, _ = self.view.rowcol(hunk_start)
-            cursor_line, _ = self.view.rowcol(cursor_pt)
-            additional_lines = cursor_line - hunk_line - 1
-
-            # Example: "diff --git a/src/js/main.spec.js b/src/js/main.spec.js" --> "src/js/main.spec.js"
-            use_prepix = re.search(r" b/(.+?)$", diff_start_line)
-            if use_prepix is None:
-                filename = diff_start_line.split(" ")[-1]
-            else:
-                filename = use_prepix.groups()[0]
-
-            # Example: "@@ -9,6 +9,7 @@" --> 9
-            lineno = int(re.search(r"^@@ \-\d+(,-?\d+)? \+(\d+)", hunk_line_str).groups()[1])
-            lineno = lineno + additional_lines
+        def first_per_file(items):
+            # type: (Iterator[Tuple[str, int, int]]) -> Iterator[Tuple[str, int, int]]
+            seen = set()  # type: Set[str]
+            for item in items:
+                filename, _, _ = item
+                if filename not in seen:
+                    seen.add(filename)
+                    yield item
 
-            self.load_file_at_line(filename, lineno)
+        diff = parse_diff_in_view(self.view)
+        jump_positions = filter_(self.jump_position_to_file(diff, pt) for pt in cursor_pts)
+        for jp in first_per_file(jump_positions):
+            self.load_file_at_line(*jp)
 
-    def load_file_at_line(self, filename, lineno):
+    def load_file_at_line(self, filename, row, col):
+        # type: (str, int, int) -> None
         """
         Show file at target commit if `git_savvy.diff_view.target_commit` is non-empty.
         Otherwise, open the file directly.
         """
         target_commit = self.view.settings().get("git_savvy.diff_view.target_commit")
         full_path = os.path.join(self.repo_path, filename)
+        window = self.view.window()
+        if not window:
+            return
+
         if target_commit:
-            self.view.window().run_command("gs_show_file_at_commit", {
+            window.run_command("gs_show_file_at_commit", {
                 "commit_hash": target_commit,
                 "filepath": full_path,
-                "lineno": lineno
+                "lineno": row,
             })
         else:
-            self.view.window().open_file(
-                "{file}:{row}:{col}".format(file=full_path, row=lineno, col=0),
+            window.open_file(
+                "{file}:{row}:{col}".format(file=full_path, row=row, col=col),
                 sublime.ENCODED_POSITION
             )
 
+    def jump_position_to_file(self, diff, pt):
+        # type: (ParsedDiff, int) -> Optional[Tuple[str, int, int]]
+        head_and_hunk_offsets = head_and_hunk_for_pt(diff, pt)
+        if not head_and_hunk_offsets:
+            return None
+
+        view = self.view
+        header_region, hunk_region = head_and_hunk_offsets
+        header = extract_content(view, header_region)
+        hunk = extract_content(view, hunk_region)
+        hunk_start, _ = hunk_region
+
+        rowcol = real_rowcol_in_hunk(hunk, relative_rowcol_in_hunk(view, hunk_start, pt))
+        if not rowcol:
+            return None
+
+        row, col = rowcol
+
+        filename = extract_filename_from_header(header)
+        if not filename:
+            return None
+
+        return filename, row, col
+
+
+def relative_rowcol_in_hunk(view, hunk_start, pt):
+    # type: (sublime.View, Point, Point) -> RowCol
+    """Return rowcol of given pt relative to hunk start"""
+    head_row, _ = view.rowcol(hunk_start)
+    row_on_pt, col = view.rowcol(pt)
+    # If `col=0` the user is on the meta char (e.g. '+- ') which is not
+    # present in the source. We pin `col` to 1 because the target API
+    # `open_file` expects 1-based row, col offsets.
+    return row_on_pt - head_row, max(col, 1)
+
+
+def real_rowcol_in_hunk(hunk, relative_rowcol):
+    # type: (str, RowCol) -> Optional[RowCol]
+    """Translate relative to absolute row, col pair"""
+    hunk_lines = split_hunk(hunk)
+    if not hunk_lines:
+        return None
+
+    row_in_hunk, col = relative_rowcol
+
+    # If the user is on the header line ('@@ ..') pretend to be on the
+    # first visible line with some content instead.
+    if row_in_hunk == 0:
+        row_in_hunk = next(
+            (
+                index
+                for index, line in enumerate(hunk_lines, 1)
+                if line.mode in ('+', ' ') and line.text.strip()
+            ),
+            1
+        )
+        col = 1
+
+    line = hunk_lines[row_in_hunk - 1]
+
+    # Happy path since the user is on a present line
+    if line.mode != '-':
+        return line.b, col
+
+    # The user is on a deleted line ('-') we cannot jump to. If possible,
+    # select the next guaranteed to be available line
+    for next_line in hunk_lines[row_in_hunk:]:
+        if next_line.mode == '+':
+            return next_line.b, min(col, len(next_line.text) + 1)
+        elif next_line.mode == ' ':
+            # If we only have a contextual line, choose this or the
+            # previous line, pretty arbitrary, depending on the
+            # indentation.
+            next_lines_indentation = line_indentation(next_line.text)
+            if next_lines_indentation == line_indentation(line.text):
+                return next_line.b, next_lines_indentation + 1
+            else:
+                return max(1, line.b - 1), 1
+    else:
+        return line.b, 1
+
+
+HUNKS_LINES_RE = re.compile(r'@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? ')
+
+
+def split_hunk(hunk):
+    # type: (str) -> Optional[List[HunkLine]]
+    """Split a hunk into (first char, line content, row) tuples
+
+    Note that rows point to available rows on the b-side.
+    """
+
+    head, *tail = hunk.rstrip().split('\n')
+    match = HUNKS_LINES_RE.search(head)
+    if not match:
+        return None
+
+    b = int(match.group(2))
+    return list(_recount_lines(tail, b))
+
+
+def _recount_lines(lines, b):
+    # type: (List[str], int) -> Iterator[HunkLine]
+
+    # Be aware that we only consider the b-line numbers, and that we
+    # always yield a b value, even for deleted lines.
+    for line in lines:
+        first_char, tail = line[0], line[1:]
+        yield HunkLine(first_char, tail, b)
+
+        if first_char != '-':
+            b += 1
+
+
+def line_indentation(line):
+    # type: (str) -> int
+    return len(line) - len(line.lstrip())
+
+
+HEADER_TO_FILE_RE = re.compile(r'\+\+\+ b/(.+)$')
+
+
+def extract_filename_from_header(header):
+    # type: (str) -> Optional[str]
+    match = HEADER_TO_FILE_RE.search(header)
+    if not match:
+        return None
+
+    return match.group(1)
+
 
 class GsDiffNavigateCommand(GsNavigate):
 

core/commands/show_commit.py

@@ -73,15 +73,20 @@ class GsShowCommitOpenFileAtHunkCommand(GsDiffOpenFileAtHunkCommand):
     and open the file at that hunk in a separate view.
     """
 
-    def load_file_at_line(self, filename, lineno):
+    def load_file_at_line(self, filename, row, col):
+        # type: (str, int, int) -> None
         """
         Show file at target commit if `git_savvy.diff_view.target_commit` is non-empty.
         Otherwise, open the file directly.
         """
         commit_hash = self.view.settings().get("git_savvy.show_commit_view.commit")
         full_path = os.path.join(self.repo_path, filename)
-        self.view.window().run_command("gs_show_file_at_commit", {
+        window = self.view.window()
+        if not window:
+            return
+
+        window.run_command("gs_show_file_at_commit", {
             "commit_hash": commit_hash,
             "filepath": full_path,
-            "lineno": lineno
+            "lineno": row
         })

tests/test_diff_view.py

@@ -231,6 +231,104 @@ def test_find_hunk_in_view(self, IN, expected):
         self.assertEqual(actual, expected)
 
 
+class TestDiffViewJumpingToFile(DeferrableTestCase):
+    @classmethod
+    def setUpClass(cls):
+        sublime.run_command("new_window")
+        cls.window = sublime.active_window()
+        s = sublime.load_settings("Preferences.sublime-settings")
+        s.set("close_windows_when_empty", False)
+
+    @classmethod
+    def tearDownClass(self):
+        self.window.run_command('close_window')
+
+    def tearDown(self):
+        unstub()
+
+    @p.expand([
+        (79, ('barz', 16, 1)),
+        (80, ('barz', 16, 1)),
+        (81, ('barz', 16, 2)),
+
+        (85, ('barz', 17, 1)),
+        (86, ('barz', 17, 2)),
+
+        # on a '-' try to select next '+' line
+        (111, ('barz', 20, 1)),  # jump to 'four'
+
+        (209, ('boox', 17, 1)),  # jump to 'thr'
+        (210, ('boox', 17, 2)),
+        (211, ('boox', 17, 3)),
+        (212, ('boox', 17, 4)),
+        (213, ('boox', 17, 1)),
+        (214, ('boox', 17, 1)),
+
+        (223, ('boox', 19, 1)),  # all jump to 'sev'
+        (228, ('boox', 19, 1)),
+        (233, ('boox', 19, 1)),
+
+        (272, ('boox', 25, 5)),
+        (280, ('boox', 25, 5)),
+
+        (319, ('boox', 30, 1)),  # but do not jump if indentation does not match
+
+        # cursor on the hunk info line selects first diff line
+        (58, ('barz', 16, 1)),
+        (59, ('barz', 16, 1)),
+        (89, ('barz', 20, 1)),
+    ])
+    def test_a(self, CURSOR, EXPECTED):
+        VIEW_CONTENT = """\
+prelude
+--
+diff --git a/fooz b/barz
+--- a/fooz
++++ b/barz
+@@ -16,1 +16,1 @@ Hi
+ one
++two
+@@ -20,1 +20,1 @@ Ho
+-three
+ context
++four
+diff --git a/foxx b/boxx
+--- a/foox
++++ b/boox
+@@ -16,1 +16,1 @@ Hello
+ one
+-two
++thr
+ fou
+-fiv
+-six
++sev
+ eig
+@@ -24 +24 @@ Hello
+     one
+-    two
+     thr
+@@ -30 +30 @@ Hello
+     one
+-    two
+ thr
+"""
+        view = self.window.new_file()
+        self.addCleanup(view.close)
+        view.run_command('append', {'characters': VIEW_CONTENT})
+        view.set_scratch(True)
+
+        cmd = module.GsDiffOpenFileAtHunkCommand(view)
+        when(cmd).load_file_at_line(...)
+
+        view.sel().clear()
+        view.sel().add(CURSOR)
+
+        cmd.run({'unused_edit'})
+
+        verify(cmd).load_file_at_line(*EXPECTED)
+
+
 class TestDiffViewHunking(DeferrableTestCase):
     @classmethod
     def setUpClass(cls):