Adding support for standalone diffs of images (#1223)

* add ImageDiff component with stub methods

* register image diff and add git widget

* handle image files in the backend

* convert binary file to base64

* return content from binary file condition

* Load image from git db

* add react-compare-image widget for viewing image diff

* remove commented out diff images

* fix formatting

* migrate to react-image-diff (not working)

* basic image diff view logic

* use mui tabs/tab components for diff modes

* change switch image diff mode logic

* add image size constraints in 2-up image diff mode

* finish minimum working onion skin image diff view

* add image width and height to the 2-up image diff view

* add file sizes to 2-up image diff view

* fix label and image border colours

* finish working swipe image diff view

* add reference and challenger indicators to the slider component, and fix styling for image and MUI components

* make sliders have the same width as the image for the swipe and onion skin image diff views

* fix order of slider labels for swipe image diff view

* fix git.py formatting

* fix ref and chall having different aspect ratios styling and behaviour, and match slider and image widths responsively

* add support for jpg and jpeg images

* fix 3 failing tests to reflect binary file support

* fix test_content_binary to reflect support for binary files

* handle empty base64 image with placeholder

* remove react-compare-image and react-image-diff dependencies

* fix image diffs from the initial commit

* Some clean ups

* Add integration test

* Lint the code

* Fix tests

---------

Co-authored-by: Damans227 <61474540+Damans227@users.noreply.github.com>
Co-authored-by: Frédéric Collonval <fcollonval@gmail.com>
Co-authored-by: Frédéric Collonval <fcollonval@users.noreply.github.com>

Author: 3 people

jupyterlab_git/git.py

@@ -1,6 +1,7 @@
 """
 Module for executing git commands, sending results back to the handlers
 """
+import base64
 import datetime
 import os
 import pathlib
@@ -51,6 +52,7 @@ async def execute(
     env: "Optional[Dict[str, str]]" = None,
     username: "Optional[str]" = None,
     password: "Optional[str]" = None,
+    is_binary=False,
 ) -> "Tuple[int, str, str]":
     """Asynchronously execute a command.
 
@@ -110,12 +112,20 @@ def call_subprocess(
         cmdline: "List[str]",
         cwd: "Optional[str]" = None,
         env: "Optional[Dict[str, str]]" = None,
+        is_binary=is_binary,
     ) -> "Tuple[int, str, str]":
         process = subprocess.Popen(
             cmdline, stdout=subprocess.PIPE, stderr=subprocess.PIPE, cwd=cwd, env=env
         )
         output, error = process.communicate()
-        return (process.returncode, output.decode("utf-8"), error.decode("utf-8"))
+        if is_binary:
+            return (
+                process.returncode,
+                base64.encodebytes(output).decode("ascii"),
+                error.decode("utf-8"),
+            )
+        else:
+            return (process.returncode, output.decode("utf-8"), error.decode("utf-8"))
 
     try:
         await execution_lock.acquire(
@@ -1325,7 +1335,7 @@ async def _get_base_ref(self, path, filename):
 
         return split_line[1] if len(split_line) > 1 else None
 
-    async def show(self, path, ref, filename=None):
+    async def show(self, path, ref, filename=None, is_binary=False):
         """
         Execute
             git show <ref:filename>
@@ -1341,7 +1351,7 @@ async def show(self, path, ref, filename=None):
         else:
             command.append(f"{ref}:{filename}")
 
-        code, output, error = await execute(command, cwd=path)
+        code, output, error = await execute(command, cwd=path, is_binary=is_binary)
 
         error_messages = map(
             lambda n: n.lower(),
@@ -1402,11 +1412,11 @@ async def get_content_at_reference(
             elif reference["special"] == "INDEX":
                 is_binary = await self._is_binary(filename, "INDEX", path)
                 if is_binary:
-                    raise tornado.web.HTTPError(
-                        log_message="Error occurred while executing command to retrieve plaintext content as file is not UTF-8."
+                    content = await self.show(
+                        path, reference["git"], filename, is_binary=True
                     )
-
-                content = await self.show(path, "", filename)
+                else:
+                    content = await self.show(path, "", filename)
             elif reference["special"] == "BASE":
                 # Special case of file in merge conflict for which we want the base (aka common ancestor) version
                 ref = await self._get_base_ref(path, filename)
@@ -1417,14 +1427,14 @@ async def get_content_at_reference(
                         reference["special"]
                     )
                 )
-        elif reference["git"]:
+        elif "git" in reference:
             is_binary = await self._is_binary(filename, reference["git"], path)
             if is_binary:
-                raise tornado.web.HTTPError(
-                    log_message="Error occurred while executing command to retrieve plaintext content as file is not UTF-8."
+                content = await self.show(
+                    path, reference["git"], filename, is_binary=True
                 )
-
-            content = await self.show(path, reference["git"], filename)
+            else:
+                content = await self.show(path, reference["git"], filename)
         else:
             content = ""
 

jupyterlab_git/tests/test_handlers.py

@@ -678,11 +678,24 @@ async def test_content(mock_execute, jp_fetch, jp_root_dir):
     assert payload["content"] == content
     mock_execute.assert_has_calls(
         [
+            call(
+                [
+                    "git",
+                    "diff",
+                    "--numstat",
+                    "4b825dc642cb6eb9a060e54bf8d69288fbee4904",
+                    "previous",
+                    "--",
+                    filename,
+                ],
+                cwd=str(local_path),
+            ),
             call(
                 ["git", "show", "{}:{}".format("previous", filename)],
                 cwd=str(local_path),
+                is_binary=False,
             ),
-        ],
+        ]
     )
 
 
@@ -779,9 +792,22 @@ async def test_content_index(mock_execute, jp_fetch, jp_root_dir):
     assert payload["content"] == content
     mock_execute.assert_has_calls(
         [
+            call(
+                [
+                    "git",
+                    "diff",
+                    "--numstat",
+                    "--cached",
+                    "4b825dc642cb6eb9a060e54bf8d69288fbee4904",
+                    "--",
+                    filename,
+                ],
+                cwd=str(local_path),
+            ),
             call(
                 ["git", "show", "{}:{}".format("", filename)],
                 cwd=str(local_path),
+                is_binary=False,
             ),
         ],
     )
@@ -824,10 +850,15 @@ async def test_content_base(mock_execute, jp_fetch, jp_root_dir):
     mock_execute.assert_has_calls(
         [
             call(
-                ["git", "show", obj_ref],
+                ["git", "ls-files", "-u", "-z", filename],
                 cwd=str(local_path),
             ),
-        ],
+            call(
+                ["git", "show", "915bb14609daab65e5304e59d89c626283ae49fc"],
+                cwd=str(local_path),
+                is_binary=False,
+            ),
+        ]
     )
 
 
@@ -902,11 +933,31 @@ async def test_content_binary(mock_execute, jp_fetch, jp_root_dir):
     }
 
     # Then
-    with pytest.raises(tornado.httpclient.HTTPClientError) as e:
-        await jp_fetch(
-            NAMESPACE, local_path.name, "content", body=json.dumps(body), method="POST"
-        )
-    assert_http_error(e, 500, expected_message="file is not UTF-8")
+    response = await jp_fetch(
+        NAMESPACE, local_path.name, "content", body=json.dumps(body), method="POST"
+    )
+
+    mock_execute.assert_has_calls(
+        [
+            call(
+                [
+                    "git",
+                    "diff",
+                    "--numstat",
+                    "4b825dc642cb6eb9a060e54bf8d69288fbee4904",
+                    "current",
+                    "--",
+                    filename,
+                ],
+                cwd=str(local_path),
+            ),
+            call(
+                ["git", "show", "{}:{}".format("current", filename)],
+                cwd=str(local_path),
+                is_binary=True,
+            ),
+        ]
+    )
 
 
 @patch("jupyterlab_git.git.execute")

package.json

@@ -77,6 +77,7 @@
     "@material-ui/lab": "^4.0.0-alpha.54",
     "@playwright/test": "^1.32.1",
     "diff-match-patch": "^1.0.4",
+    "filesize": "^10.0.7",
     "nbdime": "^6.1.1",
     "nbdime-jupyterlab": "^2.1.0",
     "react": "^17.0.1",