Merge pull request #548 from ianhi/non-ascii

Allow filenames with non-ascii characters

Author: telamonian

jupyterlab_git/git.py

@@ -101,6 +101,12 @@ def call_subprocess(
 
     return code, output, error
 
+def strip_and_split(s):
+    """strip trailing \x00 and split on \x00
+
+    Useful for parsing output of git commands with -z flag.
+    """
+    return s.strip("\x00").split("\x00")
 
 class Git:
     """
@@ -113,7 +119,7 @@ def __init__(self, contents_manager):
 
     async def config(self, top_repo_path, **kwargs):
         """Get or set Git options.
-        
+
         If no kwargs, all options are returned. Otherwise kwargs are set.
         """
         response = {"code": 1}
@@ -154,12 +160,12 @@ async def changed_files(self, base=None, remote=None, single_commit=None):
         There are two reserved "refs" for the base
             1. WORKING : Represents the Git working tree
             2. INDEX: Represents the Git staging area / index
-        
+
         Keyword Arguments:
             single_commit {string} -- The single commit ref
             base {string} -- the base Git ref
             remote {string} -- the remote Git ref
-        
+
         Returns:
             dict -- the response of format {
                 "code": int, # Command status code
@@ -168,14 +174,14 @@ async def changed_files(self, base=None, remote=None, single_commit=None):
             }
         """
         if single_commit:
-            cmd = ["git", "diff", "{}^!".format(single_commit), "--name-only"]
+            cmd = ["git", "diff", "{}^!".format(single_commit), "--name-only", "-z"]
         elif base and remote:
             if base == "WORKING":
-                cmd = ["git", "diff", remote, "--name-only"]
+                cmd = ["git", "diff", remote, "--name-only", "-z"]
             elif base == "INDEX":
-                cmd = ["git", "diff", "--staged", remote, "--name-only"]
+                cmd = ["git", "diff", "--staged", remote, "--name-only", "-z"]
             else:
-                cmd = ["git", "diff", base, remote, "--name-only"]
+                cmd = ["git", "diff", base, remote, "--name-only", "-z"]
         else:
             raise tornado.web.HTTPError(
                 400, "Either single_commit or (base and remote) must be provided"
@@ -193,7 +199,7 @@ async def changed_files(self, base=None, remote=None, single_commit=None):
                 response["command"] = " ".join(cmd)
                 response["message"] = error
             else:
-                response["files"] = output.strip().split("\n")
+                response["files"] = strip_and_split(output)
 
         return response
 
@@ -236,7 +242,7 @@ async def status(self, current_path):
         """
         Execute git status command & return the result.
         """
-        cmd = ["git", "status", "--porcelain", "-u"]
+        cmd = ["git", "status", "--porcelain", "-u", "-z"]
         code, my_output, my_error = await execute(
             cmd, cwd=os.path.join(self.root_dir, current_path),
         )
@@ -249,20 +255,15 @@ async def status(self, current_path):
             }
 
         result = []
-        line_array = my_output.splitlines()
-        for line in line_array:
-            to1 = None
-            from_path = line[3:]
-            if line[0] == "R":
-                to0 = line[3:].split(" -> ")
-                to1 = to0[len(to0) - 1]
-            else:
-                to1 = line[3:]
-            if to1.startswith('"'):
-                to1 = to1[1:]
-            if to1.endswith('"'):
-                to1 = to1[:-1]
-            result.append({"x": line[0], "y": line[1], "to": to1, "from": from_path})
+        line_iterable = iter(strip_and_split(my_output))
+        for line in line_iterable:
+            result.append({
+                "x": line[0],
+                "y": line[1],
+                "to": line[3:],
+                # if file was renamed, next line contains original path
+                "from": next(line_iterable) if line[0]=='R' else line[3:]
+            })       
         return {"code": code, "files": result}
 
     async def log(self, current_path, history_count=10):
@@ -311,73 +312,69 @@ async def log(self, current_path, history_count=10):
 
     async def detailed_log(self, selected_hash, current_path):
         """
-        Execute git log -1 --stat --numstat --oneline command (used to get
+        Execute git log -1 --numstat --oneline -z command (used to get
         insertions & deletions per file) & return the result.
         """
-        cmd = ["git", "log", "-1", "--stat", "--numstat", "--oneline", selected_hash]
+        cmd = ["git", "log", "-1", "--numstat", "--oneline", "-z", selected_hash]
         code, my_output, my_error = await execute(
             cmd, cwd=os.path.join(self.root_dir, current_path),
         )
 
         if code != 0:
             return {"code": code, "command": " ".join(cmd), "message": my_error}
 
+        total_insertions = 0
+        total_deletions = 0
         result = []
-        note = [0] * 3
-        count = 0
-        temp = ""
-        line_array = my_output.splitlines()
-        length = len(line_array)
-        INSERTION_INDEX = 0
-        DELETION_INDEX = 1
-        MODIFIED_FILE_PATH_INDEX = 2
-        if length > 1:
-            temp = line_array[length - 1]
-            words = temp.split()
-            for i in range(0, len(words)):
-                if words[i].isdigit():
-                    note[count] = words[i]
-                    count += 1
-            for num in range(1, int(length / 2)):
-                line_info = line_array[num].split(maxsplit=2)
-                words = line_info[2].split("/")
-                length = len(words)
-                result.append(
-                    {
-                        "modified_file_path": line_info[MODIFIED_FILE_PATH_INDEX],
-                        "modified_file_name": words[length - 1],
-                        "insertion": line_info[INSERTION_INDEX],
-                        "deletion": line_info[DELETION_INDEX],
-                    }
-                )
-
-        if note[2] == 0 and length > 1:
-            if "-" in temp:
-                exchange = note[1]
-                note[1] = note[2]
-                note[2] = exchange
+        line_iterable = iter(strip_and_split(my_output)[1:])
+        for line in line_iterable:
+            insertions, deletions, file = line.split('\t')
+
+            if file == '':
+                # file was renamed or moved, we need next two lines of output
+                from_path = next(line_iterable)
+                to_path = next(line_iterable)
+                modified_file_name = from_path + " => " + to_path
+                modified_file_path = to_path
+            else:
+                modified_file_name = file.split("/")[-1]
+                modified_file_path = file
+
+            result.append({
+                        "modified_file_path": modified_file_path,
+                        "modified_file_name": modified_file_name,
+                        "insertion": insertions,
+                        "deletion": deletions,
+            })
+            total_insertions += int(insertions)
+            total_deletions += int(deletions)
+
+        modified_file_note = "{num_files} files changed, {insertions} insertions(+), {deletions} deletions(-)".format(
+            num_files=len(result),
+            insertions=total_insertions,
+            deletions=total_deletions)
 
         return {
             "code": code,
-            "modified_file_note": temp,
-            "modified_files_count": note[0],
-            "number_of_insertions": note[1],
-            "number_of_deletions": note[2],
+            "modified_file_note": modified_file_note,
+            "modified_files_count": str(len(result)),
+            "number_of_insertions": str(total_insertions),
+            "number_of_deletions": str(total_deletions),
             "modified_files": result,
         }
 
     async def diff(self, top_repo_path):
         """
         Execute git diff command & return the result.
         """
-        cmd = ["git", "diff", "--numstat"]
+        cmd = ["git", "diff", "--numstat", "-z"]
         code, my_output, my_error = await execute(cmd, cwd=top_repo_path)
 
         if code != 0:
             return {"code": code, "command": " ".join(cmd), "message": my_error}
 
         result = []
-        line_array = my_output.splitlines()
+        line_array = strip_and_split(my_output)
         for line in line_array:
             linesplit = line.split()
             result.append(

jupyterlab_git/tests/test_detailed_log.py

@@ -17,26 +17,27 @@ async def test_detailed_log():
         # Given
         process_output = [
             "f29660a (HEAD, origin/feature) Commit message",
-            "10      3       notebook_without_spaces.ipynb",
-            "11      4       Notebook with spaces.ipynb",
-            "12      5       path/notebook_without_spaces.ipynb",
-            "13      6       path/Notebook with spaces.ipynb",
-            " notebook_without_spaces.ipynb      | 13 ++++++++---",
-            " Notebook with spaces.ipynb         | 15 +++++++++----",
-            " path/notebook_without_spaces.ipynb | 17 ++++++++++-----",
-            " path/Notebook with spaces.ipynb    | 19 +++++++++++------",
-            " 4 files changed, 46 insertions(+), 18 deletions(-)",
+            "10\t3\tnotebook_without_spaces.ipynb",
+            "11\t4\tNotebook with spaces.ipynb",
+            "12\t5\tpath/notebook_without_spaces.ipynb",
+            "13\t6\tpath/Notebook with spaces.ipynb",
+            "14\t1\tpath/Notebook with λ.ipynb",
+            "0\t0\t",
+            "folder1/file with spaces and λ.py",
+            "folder2/file with spaces.py"
         ]
-        mock_execute.return_value = tornado.gen.maybe_future(
-            (0, "\n".join(process_output), "")
+
+
+        mock_execute._mock_return_value = tornado.gen.maybe_future(
+            (0, "\x00".join(process_output), "")
         )
 
         expected_response = {
             "code": 0,
-            "modified_file_note": " 4 files changed, 46 insertions(+), 18 deletions(-)",
-            "modified_files_count": "4",
-            "number_of_insertions": "46",
-            "number_of_deletions": "18",
+            "modified_file_note": "6 files changed, 60 insertions(+), 19 deletions(-)",
+            "modified_files_count": "6",
+            "number_of_insertions": "60",
+            "number_of_deletions": "19",
             "modified_files": [
                 {
                     "modified_file_path": "notebook_without_spaces.ipynb",
@@ -62,11 +63,23 @@ async def test_detailed_log():
                     "insertion": "13",
                     "deletion": "6",
                 },
+                {
+                    "modified_file_path": "path/Notebook with λ.ipynb",
+                    "modified_file_name": "Notebook with λ.ipynb",
+                    "insertion": "14",
+                    "deletion": "1",
+                },
+                {
+                    "modified_file_path": "folder2/file with spaces.py",
+                    "modified_file_name": "folder1/file with spaces and λ.py => folder2/file with spaces.py",
+                    "insertion": "0",
+                    "deletion": "0",
+                },
             ],
         }
 
         # When
-        actual_response = (await 
+        actual_response = (await
             Git(FakeContentManager("/bin"))
             .detailed_log(
                 selected_hash="f29660a2472e24164906af8653babeb48e4bf2ab",
@@ -80,9 +93,9 @@ async def test_detailed_log():
                 "git",
                 "log",
                 "-1",
-                "--stat",
                 "--numstat",
                 "--oneline",
+                "-z",
                 "f29660a2472e24164906af8653babeb48e4bf2ab",
             ],
             cwd=os.path.join("/bin", "test_curr_path"),

jupyterlab_git/tests/test_diff.py

@@ -24,7 +24,7 @@ async def test_changed_files_single_commit():
     with patch("jupyterlab_git.git.execute") as mock_execute:
         # Given
         mock_execute.return_value = tornado.gen.maybe_future(
-            (0, "file1.ipynb\nfile2.py", "")
+            (0, "file1.ipynb\x00file2.py", "")
         )
 
         # When
@@ -39,6 +39,7 @@ async def test_changed_files_single_commit():
                 "diff",
                 "64950a634cd11d1a01ddfedaeffed67b531cb11e^!",
                 "--name-only",
+                "-z",
             ],
             cwd="/bin",
         )
@@ -50,7 +51,7 @@ async def test_changed_files_working_tree():
     with patch("jupyterlab_git.git.execute") as mock_execute:
         # Given
         mock_execute.return_value = tornado.gen.maybe_future(
-            (0, "file1.ipynb\nfile2.py", "")
+            (0, "file1.ipynb\x00file2.py", "")
         )
 
         # When
@@ -60,7 +61,7 @@ async def test_changed_files_working_tree():
 
         # Then
         mock_execute.assert_called_once_with(
-            ["git", "diff", "HEAD", "--name-only"], cwd="/bin"
+            ["git", "diff", "HEAD", "--name-only", "-z"], cwd="/bin"
         )
         assert {"code": 0, "files": ["file1.ipynb", "file2.py"]} == actual_response
 
@@ -70,7 +71,7 @@ async def test_changed_files_index():
     with patch("jupyterlab_git.git.execute") as mock_execute:
         # Given
         mock_execute.return_value = tornado.gen.maybe_future(
-            (0, "file1.ipynb\nfile2.py", "")
+            (0, "file1.ipynb\x00file2.py", "")
         )
 
         # When
@@ -80,7 +81,7 @@ async def test_changed_files_index():
 
         # Then
         mock_execute.assert_called_once_with(
-            ["git", "diff", "--staged", "HEAD", "--name-only"], cwd="/bin"
+            ["git", "diff", "--staged", "HEAD", "--name-only", "-z"], cwd="/bin"
         )
         assert {"code": 0, "files": ["file1.ipynb", "file2.py"]} == actual_response
 
@@ -90,7 +91,7 @@ async def test_changed_files_two_commits():
     with patch("jupyterlab_git.git.execute") as mock_execute:
         # Given
         mock_execute.return_value = tornado.gen.maybe_future(
-            (0, "file1.ipynb\nfile2.py", "")
+            (0, "file1.ipynb\x00file2.py", "")
         )
 
         # When
@@ -100,7 +101,7 @@ async def test_changed_files_two_commits():
 
         # Then
         mock_execute.assert_called_once_with(
-            ["git", "diff", "HEAD", "origin/HEAD", "--name-only"], cwd="/bin"
+            ["git", "diff", "HEAD", "origin/HEAD", "--name-only", "-z"], cwd="/bin"
         )
         assert {"code": 0, "files": ["file1.ipynb", "file2.py"]} == actual_response
 
@@ -118,6 +119,6 @@ async def test_changed_files_git_diff_error():
 
         # Then
         mock_execute.assert_called_once_with(
-            ["git", "diff", "HEAD", "origin/HEAD", "--name-only"], cwd="/bin"
+            ["git", "diff", "HEAD", "origin/HEAD", "--name-only", "-z"], cwd="/bin"
         )
         assert {"code": 128, "message": "error message"} == actual_response