Merge pull request #506 from fcollonval/498-fix-loading-config

Fix multiline Git options interpretation

Author: telamonian

jupyterlab_git/git.py

@@ -2,6 +2,7 @@
 Module for executing git commands, sending results back to the handlers
 """
 import os
+import re
 import subprocess
 from subprocess import Popen, PIPE, CalledProcessError
 
@@ -10,7 +11,11 @@
 from tornado.web import HTTPError
 
 
+# Git configuration options exposed through the REST API
 ALLOWED_OPTIONS = ['user.name', 'user.email']
+# Regex pattern to capture (key, value) of Git configuration options.
+# See https://git-scm.com/docs/git-config#_syntax for git var syntax
+CONFIG_PATTERN = re.compile(r"(?:^|\n)([\w\-\.]+)\=")
 
 
 class GitAuthInputWrapper:
@@ -102,11 +107,8 @@ def config(self, top_repo_path, **kwargs):
                 response["message"] = error.decode("utf-8").strip()
             else:
                 raw = output.decode("utf-8").strip()
-                response["options"] = dict()
-                for l in raw.split("\n"):
-                    k, v = l.split("=", maxsplit=1)
-                    if k in ALLOWED_OPTIONS:
-                        response["options"][k] = v
+                s = CONFIG_PATTERN.split(raw)
+                response["options"] = {k:v for k, v in zip(s[1::2], s[2::2]) if k in ALLOWED_OPTIONS}
 
         return response
 

jupyterlab_git/tests/test_config.py

@@ -50,6 +50,110 @@ def test_git_get_config_success(self, popen):
             },
         }
 
+    @patch("subprocess.Popen")
+    def test_git_get_config_multiline(self, popen):
+        # Given
+        process_mock = Mock()
+        attrs = {
+            "communicate": Mock(
+                return_value=(
+                    b"user.name=John Snow\n"
+                    b"user.email=john.snow@iscoming.com\n"
+                    b'alias.summary=!f() {     printf "Summary of this branch...\n'
+                    b'";     printf "%s\n'
+                    b'" $(git rev-parse --abbrev-ref HEAD);     printf "\n'
+                    b"Most-active files, with churn count\n"
+                    b'"; git churn | head -7;   }; f\n'
+                    b'alias.topic-base-branch-name=!f(){     printf "master\n'
+                    b'";   };f\n'
+                    b'alias.topic-start=!f(){     topic_branch="$1";     git topic-create "$topic_branch";     git topic-push;   };f',
+                    b"",
+                )
+            ),
+            "returncode": 0,
+        }
+        process_mock.configure_mock(**attrs)
+        popen.return_value = process_mock
+
+        # When
+        body = {"path": "test_path"}
+        response = self.tester.post(["config"], body=body)
+
+        # Then
+        popen.assert_called_once_with(
+            ["git", "config", "--list"],
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE,
+            cwd="test_path",
+        )
+        process_mock.communicate.assert_called_once_with()
+
+        assert response.status_code == 201
+        payload = response.json()
+        assert payload == {
+            "code": 0,
+            "options": {
+                "user.name": "John Snow",
+                "user.email": "john.snow@iscoming.com",
+            },
+        }
+
+    @patch("subprocess.Popen")
+    @patch(
+        "jupyterlab_git.git.ALLOWED_OPTIONS",
+        ["alias.summary", "alias.topic-base-branch-name"],
+    )
+    def test_git_get_config_accepted_multiline(self, popen):
+        # Given
+        process_mock = Mock()
+        attrs = {
+            "communicate": Mock(
+                return_value=(
+                    b"user.name=John Snow\n"
+                    b"user.email=john.snow@iscoming.com\n"
+                    b'alias.summary=!f() {     printf "Summary of this branch...\n'
+                    b'";     printf "%s\n'
+                    b'" $(git rev-parse --abbrev-ref HEAD);     printf "\n'
+                    b"Most-active files, with churn count\n"
+                    b'"; git churn | head -7;   }; f\n'
+                    b'alias.topic-base-branch-name=!f(){     printf "master\n'
+                    b'";   };f\n'
+                    b'alias.topic-start=!f(){     topic_branch="$1";     git topic-create "$topic_branch";     git topic-push;   };f',
+                    b"",
+                )
+            ),
+            "returncode": 0,
+        }
+        process_mock.configure_mock(**attrs)
+        popen.return_value = process_mock
+
+        # When
+        body = {"path": "test_path"}
+        response = self.tester.post(["config"], body=body)
+
+        # Then
+        popen.assert_called_once_with(
+            ["git", "config", "--list"],
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE,
+            cwd="test_path",
+        )
+        process_mock.communicate.assert_called_once_with()
+
+        assert response.status_code == 201
+        payload = response.json()
+        assert payload == {
+            "code": 0,
+            "options": {
+                "alias.summary": '!f() {     printf "Summary of this branch...\n'
+                '";     printf "%s\n'
+                '" $(git rev-parse --abbrev-ref HEAD);     printf "\n'
+                "Most-active files, with churn count\n"
+                '"; git churn | head -7;   }; f',
+                "alias.topic-base-branch-name": '!f(){     printf "master\n";   };f',
+            },
+        }
+
     @patch("subprocess.Popen")
     def test_git_set_config_success(self, popen):
         # Given