Merge pull request #586 from jupyterlab/585-fix-staging-diff

Fix staging diff

Author: fcollonval

jupyterlab_git/git.py

@@ -281,15 +281,36 @@ async def status(self, current_path):
                 "message": my_error,
             }
 
+        # Add attribute `is_binary`
+        command = [  # Compare stage to an empty tree see `_is_binary`
+            "git",
+            "diff",
+            "--numstat",
+            "-z",
+            "--cached",
+            "4b825dc642cb6eb9a060e54bf8d69288fbee4904"
+        ]
+        text_code, text_output, _ = await execute(
+            command, cwd=os.path.join(self.root_dir, current_path),
+        )
+
+        are_binary = dict()
+        if text_code == 0:
+            for line in filter(lambda l: len(l) > 0, strip_and_split(text_output)):
+                diff, name = line.rsplit("\t", maxsplit=1)
+                are_binary[name] = diff.startswith("-\t-")
+        
         result = []
         line_iterable = (line for line in strip_and_split(my_output) if line)
         for line in line_iterable:
+            name = line[3:]
             result.append({
                 "x": line[0],
                 "y": line[1],
-                "to": line[3:],
+                "to": name,
                 # if file was renamed, next line contains original path
-                "from": next(line_iterable) if line[0]=='R' else line[3:]
+                "from": next(line_iterable) if line[0]=='R' else name,
+                "is_binary": are_binary.get(name, None)
             })
         return {"code": code, "files": result}
 
@@ -355,6 +376,7 @@ async def detailed_log(self, selected_hash, current_path):
         result = []
         line_iterable = iter(strip_and_split(my_output)[1:])
         for line in line_iterable:
+            is_binary = line.startswith("-\t-\t")
             insertions, deletions, file = line.split('\t')
             insertions = insertions.replace('-', '0')
             deletions = deletions.replace('-', '0')
@@ -370,10 +392,11 @@ async def detailed_log(self, selected_hash, current_path):
                 modified_file_path = file
 
             result.append({
-                        "modified_file_path": modified_file_path,
-                        "modified_file_name": modified_file_name,
-                        "insertion": insertions,
-                        "deletion": deletions,
+                "modified_file_path": modified_file_path,
+                "modified_file_name": modified_file_name,
+                "insertion": insertions,
+                "deletion": deletions,
+                "is_binary": is_binary
             })
             total_insertions += int(insertions)
             total_deletions += int(deletions)
@@ -1003,7 +1026,7 @@ async def diff_content(self, filename, prev_ref, curr_ref, top_repo_path):
             if curr_ref["special"] == "WORKING":
                 curr_content = self.get_content(filename, top_repo_path)
             elif curr_ref["special"] == "INDEX":
-                is_binary = await self._is_binary(filename, "", top_repo_path)
+                is_binary = await self._is_binary(filename, "INDEX", top_repo_path)
                 if is_binary:
                     raise tornado.web.HTTPError(log_message="Error occurred while executing command to retrieve plaintext diff as file is not UTF-8.")
 
@@ -1030,8 +1053,22 @@ async def _is_binary(self, filename, ref, top_repo_path):
         -   <https://stackoverflow.com/questions/6119956/how-to-determine-if-git-handles-a-file-as-binary-or-as-text/6134127#6134127>
         -   <https://git-scm.com/docs/git-diff#Documentation/git-diff.txt---numstat>
         -   <https://git-scm.com/docs/git-diff#_other_diff_formats>
+
+        Args:
+            filename (str): Filename (relative to the git repository)
+            ref (str): Commit reference or "INDEX" if file is staged
+            top_repo_path (str): Git repository filepath
+
+        Returns:
+            bool: Is file binary?
+        
+        Raises:
+            HTTPError: if git command failed
         """
-        command = ["git", "diff", "--numstat", "4b825dc642cb6eb9a060e54bf8d69288fbee4904", ref, "--", filename]  # where 4b825... is a magic SHA which represents the empty tree
+        if ref == "INDEX":
+            command = ["git", "diff", "--numstat", "--cached", "4b825dc642cb6eb9a060e54bf8d69288fbee4904", "--", filename]
+        else:
+            command = ["git", "diff", "--numstat", "4b825dc642cb6eb9a060e54bf8d69288fbee4904", ref, "--", filename]  # where 4b825... is a magic SHA which represents the empty tree
         code, output, error = await execute(command, cwd=top_repo_path)
 
         if code != 0:
@@ -1042,10 +1079,7 @@ async def _is_binary(self, filename, ref, top_repo_path):
             raise tornado.web.HTTPError(log_message="Error while determining if file is binary or text '{}'.".format(error))
 
         # For binary files, `--numstat` outputs two `-` characters separated by TABs:
-        if output.startswith('-\t-\t'):
-            return True
-
-        return False
+        return output.startswith('-\t-\t')
 
     def remote_add(self, top_repo_path, url, name=DEFAULT_REMOTE_NAME):
         """Handle call to `git remote add` command.

jupyterlab_git/tests/test_branch.py

@@ -8,7 +8,7 @@
 # local lib
 from jupyterlab_git.git import Git
 
-from .testutils import FakeContentManager
+from .testutils import FakeContentManager, maybe_future
 
 
 def test_is_remote_branch():
@@ -33,7 +33,7 @@ async def test_get_current_branch_success():
 
     with patch("jupyterlab_git.git.execute") as mock_execute:
         # Given
-        mock_execute.return_value = tornado.gen.maybe_future((0, "feature-foo", ""))
+        mock_execute.return_value = maybe_future((0, "feature-foo", ""))
 
         # When
         actual_response = await (
@@ -59,10 +59,10 @@ async def test_checkout_branch_noref_success():
 
     with patch("jupyterlab_git.git.execute") as mock_execute:
         with patch.object(
-            Git, "_get_branch_reference", return_value=tornado.gen.maybe_future(None)
+            Git, "_get_branch_reference", return_value=maybe_future(None)
         ) as mock__get_branch_reference:
             # Given
-            mock_execute.return_value = tornado.gen.maybe_future(
+            mock_execute.return_value = maybe_future(
                 (rc, stdout_message, stderr_message)
             )
 
@@ -93,10 +93,10 @@ async def test_checkout_branch_noref_failure():
     rc = 1
     with patch("jupyterlab_git.git.execute") as mock_execute:
         with patch.object(
-            Git, "_get_branch_reference", return_value=tornado.gen.maybe_future(None)
+            Git, "_get_branch_reference", return_value=maybe_future(None)
         ) as mock__get_branch_reference:
             # Given
-            mock_execute.return_value = tornado.gen.maybe_future(
+            mock_execute.return_value = maybe_future(
                 (rc, stdout_message, stderr_message)
             )
 
@@ -132,10 +132,10 @@ async def test_checkout_branch_remoteref_success():
         with patch.object(
             Git,
             "_get_branch_reference",
-            return_value=tornado.gen.maybe_future("refs/remotes/remote_branch"),
+            return_value=maybe_future("refs/remotes/remote_branch"),
         ) as mock__get_branch_reference:
             # Given
-            mock_execute.return_value = tornado.gen.maybe_future(
+            mock_execute.return_value = maybe_future(
                 (rc, stdout_message, stderr_message)
             )
 
@@ -169,10 +169,10 @@ async def test_checkout_branch_headsref_failure():
         with patch.object(
             Git,
             "_get_branch_reference",
-            return_value=tornado.gen.maybe_future("refs/heads/local_branch"),
+            return_value=maybe_future("refs/heads/local_branch"),
         ) as mock__get_branch_reference:
             # Given
-            mock_execute.return_value = tornado.gen.maybe_future(
+            mock_execute.return_value = maybe_future(
                 (rc, stdout_message, stderr_message)
             )
 
@@ -206,10 +206,10 @@ async def test_checkout_branch_headsref_success():
         with patch.object(
             Git,
             "_get_branch_reference",
-            return_value=tornado.gen.maybe_future("refs/heads/local_branch"),
+            return_value=maybe_future("refs/heads/local_branch"),
         ):
             # Given
-            mock_execute.return_value = tornado.gen.maybe_future(
+            mock_execute.return_value = maybe_future(
                 (rc, stdout_message, stderr_message)
             )
 
@@ -239,10 +239,10 @@ async def test_checkout_branch_remoteref_failure():
         with patch.object(
             Git,
             "_get_branch_reference",
-            return_value=tornado.gen.maybe_future("refs/remotes/remote_branch"),
+            return_value=maybe_future("refs/remotes/remote_branch"),
         ):
             # Given
-            mock_execute.return_value = tornado.gen.maybe_future(
+            mock_execute.return_value = maybe_future(
                 (rc, stdout_message, stderr_message)
             )
 
@@ -272,7 +272,7 @@ async def test_get_branch_reference_success():
         branch = "test-branch"
         reference = "refs/remotes/origin/test_branch"
 
-        mock_execute.return_value = tornado.gen.maybe_future((0, reference, ""))
+        mock_execute.return_value = maybe_future((0, reference, ""))
 
         # When
         actual_response = await Git(FakeContentManager("/bin"))._get_branch_reference(
@@ -294,7 +294,7 @@ async def test_get_branch_reference_failure():
         branch = "test-branch"
         reference = "test-branch"
         # Given
-        mock_execute.return_value = tornado.gen.maybe_future(
+        mock_execute.return_value = maybe_future(
             (
                 128,
                 reference,
@@ -321,7 +321,7 @@ async def test_get_branch_reference_failure():
 async def test_get_current_branch_failure():
     with patch("jupyterlab_git.git.execute") as mock_execute:
         # Given
-        mock_execute.return_value = tornado.gen.maybe_future(
+        mock_execute.return_value = maybe_future(
             (
                 128,
                 "",
@@ -356,7 +356,7 @@ async def test_get_current_branch_detached_success():
             "  remotes/origin/feature-foo",
             "  remotes/origin/HEAD",
         ]
-        mock_execute.return_value = tornado.gen.maybe_future(
+        mock_execute.return_value = maybe_future(
             (0, "\n".join(process_output), "")
         )
 
@@ -376,7 +376,7 @@ async def test_get_current_branch_detached_success():
 async def test_get_current_branch_detached_failure():
     with patch("jupyterlab_git.git.execute") as mock_execute:
         # Given
-        mock_execute.return_value = tornado.gen.maybe_future(
+        mock_execute.return_value = maybe_future(
             (
                 128,
                 "",
@@ -413,7 +413,7 @@ async def test_get_current_branch_detached_failure():
 async def test_get_upstream_branch_success(branch, upstream):
     with patch("jupyterlab_git.git.execute") as mock_execute:
         # Given
-        mock_execute.return_value = tornado.gen.maybe_future((0, upstream, ""))
+        mock_execute.return_value = maybe_future((0, upstream, ""))
 
         # When
         actual_response = await Git(FakeContentManager("/bin")).get_upstream_branch(
@@ -451,7 +451,7 @@ async def test_get_upstream_branch_success(branch, upstream):
 async def test_get_upstream_branch_failure(outputs, message):
     with patch("jupyterlab_git.git.execute") as mock_execute:
         # Given
-        mock_execute.return_value = tornado.gen.maybe_future(outputs)
+        mock_execute.return_value = maybe_future(outputs)
 
         # When
         if message:
@@ -482,7 +482,7 @@ async def test_get_upstream_branch_failure(outputs, message):
 async def test_get_tag_success():
     with patch("jupyterlab_git.git.execute") as mock_execute:
         # Given
-        mock_execute.return_value = tornado.gen.maybe_future((0, "v0.3.0", ""))
+        mock_execute.return_value = maybe_future((0, "v0.3.0", ""))
 
         # When
         actual_response = await Git(FakeContentManager("/bin"))._get_tag(
@@ -503,8 +503,8 @@ async def test_get_tag_failure():
     with patch("jupyterlab_git.git.execute") as mock_execute:
         # Given
         mock_execute.side_effect = [
-            tornado.gen.maybe_future((128, "", "fatal: Not a valid object name blah")),
-            tornado.gen.maybe_future(
+            maybe_future((128, "", "fatal: Not a valid object name blah")),
+            maybe_future(
                 (
                     128,
                     "",
@@ -557,7 +557,7 @@ async def test_get_tag_failure():
 async def test_no_tags():
     with patch("jupyterlab_git.git.execute") as mock_execute:
         # Given
-        mock_execute.return_value = tornado.gen.maybe_future(
+        mock_execute.return_value = maybe_future(
             (128, "", "fatal: No names found, cannot describe anything.\n")
         )
 
@@ -590,9 +590,9 @@ async def test_branch_success():
 
         mock_execute.side_effect = [
             # Response for get all refs/heads
-            tornado.gen.maybe_future((0, "\n".join(process_output_heads), "")),
+            maybe_future((0, "\n".join(process_output_heads), "")),
             # Response for get all refs/remotes
-            tornado.gen.maybe_future((0, "\n".join(process_output_remotes), "")),
+            maybe_future((0, "\n".join(process_output_remotes), "")),
         ]
 
         expected_response = {
@@ -694,7 +694,7 @@ async def test_branch_failure():
             "--format=%(refname:short)%09%(objectname)%09%(upstream:short)%09%(HEAD)",
             "refs/heads/",
         ]
-        mock_execute.return_value = tornado.gen.maybe_future(
+        mock_execute.return_value = maybe_future(
             (
                 128,
                 "",
@@ -738,15 +738,15 @@ async def test_branch_success_detached_head():
 
         mock_execute.side_effect = [
             # Response for get all refs/heads
-            tornado.gen.maybe_future((0, "\n".join(process_output_heads), "")),
+            maybe_future((0, "\n".join(process_output_heads), "")),
             # Response for get current branch
-            tornado.gen.maybe_future(
+            maybe_future(
                 (128, "", "fatal: ref HEAD is not a symbolic ref")
             ),
             # Response for get current branch detached
-            tornado.gen.maybe_future((0, "\n".join(detached_head_output), "")),
+            maybe_future((0, "\n".join(detached_head_output), "")),
             # Response for get all refs/remotes
-            tornado.gen.maybe_future((0, "\n".join(process_output_remotes), "")),
+            maybe_future((0, "\n".join(process_output_remotes), "")),
         ]
 
         expected_response = {

jupyterlab_git/tests/test_clone.py

@@ -6,15 +6,15 @@
 
 from jupyterlab_git.git import Git
 
-from .testutils import FakeContentManager
+from .testutils import FakeContentManager, maybe_future
 
 
 @pytest.mark.asyncio
 async def test_git_clone_success():
     with patch("os.environ", {"TEST": "test"}):
         with patch("jupyterlab_git.git.execute") as mock_execute:
             # Given
-            mock_execute.return_value = tornado.gen.maybe_future((0, "output", "error"))
+            mock_execute.return_value = maybe_future((0, "output", "error"))
 
             # When
             actual_response = await Git(FakeContentManager("/bin")).clone(
@@ -40,7 +40,7 @@ async def test_git_clone_failure_from_git():
     with patch("os.environ", {"TEST": "test"}):
         with patch("jupyterlab_git.git.execute") as mock_execute:
             # Given
-            mock_execute.return_value = tornado.gen.maybe_future(
+            mock_execute.return_value = maybe_future(
                 (128, "test_output", "fatal: Not a git repository")
             )
 
@@ -66,7 +66,7 @@ async def test_git_clone_with_auth_success():
     with patch("os.environ", {"TEST": "test"}):
         with patch("jupyterlab_git.git.execute") as mock_authentication:
             # Given
-            mock_authentication.return_value = tornado.gen.maybe_future((0, "", ""))
+            mock_authentication.return_value = maybe_future((0, "", ""))
 
             # When
             auth = {"username": "asdf", "password": "qwerty"}
@@ -95,7 +95,7 @@ async def test_git_clone_with_auth_wrong_repo_url_failure_from_git():
     with patch("os.environ", {"TEST": "test"}):
         with patch("jupyterlab_git.git.execute") as mock_authentication:
             # Given
-            mock_authentication.return_value = tornado.gen.maybe_future(
+            mock_authentication.return_value = maybe_future(
                 (128, "", "fatal: repository 'ghjkhjkl' does not exist")
             )
 
@@ -129,7 +129,7 @@ async def test_git_clone_with_auth_auth_failure_from_git():
     with patch("os.environ", {"TEST": "test"}):
         with patch("jupyterlab_git.git.execute") as mock_authentication:
             # Given
-            mock_authentication.return_value = tornado.gen.maybe_future(
+            mock_authentication.return_value = maybe_future(
                 (
                     128,
                     "",