Cleaned up passing of branch on query string

The following changes ensure branch is only passed when it has been
defined:
- branch becomes keyword parameter to GitPuller
- branch only addd to query string when not undefined
- branch only encoded into url when exists

Author: sean-morris

nbgitpuller/handlers.py

@@ -52,7 +52,7 @@ def get(self):
 
         try:
             repo = self.get_argument('repo')
-            branch = self.get_argument('branch')
+            branch = self.get_argument('branch', None)
             depth = self.get_argument('depth', None)
             if depth:
                 depth = int(depth)
@@ -73,7 +73,7 @@ def get(self):
             self.set_header('content-type', 'text/event-stream')
             self.set_header('cache-control', 'no-cache')
 
-            gp = GitPuller(repo, branch, repo_dir, depth=depth, parent=self.settings['nbapp'])
+            gp = GitPuller(repo, repo_dir, branch=branch, depth=depth, parent=self.settings['nbapp'])
 
             q = Queue()
 

nbgitpuller/pull.py

@@ -66,17 +66,16 @@ def _depth_default(self):
         where the GitPuller class hadn't been loaded already."""
         return int(os.environ.get('NBGITPULLER_DEPTH', 1))
 
-    def __init__(self, git_url, branch_name, repo_dir, **kwargs):
-        assert git_url and branch_name
+    def __init__(self, git_url, repo_dir, **kwargs):
+        assert git_url
 
         self.git_url = git_url
+        self.branch_name = kwargs.pop("branch")
 
-        if branch_name == "None":
+        if self.branch_name is None:
             self.branch_name = self.resolve_default_branch()
-        elif not self.branch_exists(branch_name):
-            raise ValueError(f"{branch_name}: branch not found in {self.git_url}")
-        else:
-            self.branch_name = branch_name
+        elif not self.branch_exists(self.branch_name):
+            raise ValueError(f"Branch: {self.branch_name} -- not found in repo: {self.git_url}")
 
         self.repo_dir = repo_dir
         newargs = {k: v for k, v in kwargs.items() if v is not None}
@@ -304,13 +303,11 @@ def main():
 
     parser = argparse.ArgumentParser(description='Synchronizes a github repository with a local repository.')
     parser.add_argument('git_url', help='Url of the repo to sync')
-    parser.add_argument('branch_name', default='master', help='Branch of repo to sync', nargs='?')
     parser.add_argument('repo_dir', default='.', help='Path to clone repo under', nargs='?')
     args = parser.parse_args()
 
     for line in GitPuller(
         args.git_url,
-        args.branch_name,
         args.repo_dir
     ).pull():
         print(line)

nbgitpuller/static/index.js

@@ -44,12 +44,14 @@ require([
         // Start git pulling handled by SyncHandler, declared in handlers.py
         var syncUrlParams = {
             repo: this.repo,
-            branch: this.branch,
             targetpath: this.targetpath
         }
         if (typeof this.depth !== 'undefined' && this.depth != undefined) {
             syncUrlParams['depth'] = this.depth;
         }
+        if (typeof this.branch !== 'undefined' && this.branch != undefined) {
+            syncUrlParams['branch'] = this.branch;
+        }
         var syncUrl = this.baseUrl + 'git-pull/api?' + $.param(syncUrlParams);
 
         this.eventSource = new EventSource(syncUrl);

nbgitpuller/templates/status.html

@@ -5,7 +5,7 @@
 data-base-url="{{ base_url | urlencode }}"
 data-repo="{{ repo | urlencode }}"
 data-path="{{ path | urlencode }}"
-data-branch="{{ branch | urlencode }}"
+{% if branch %}data-branch="{{ branch | urlencode }}"{% endif %}
 {% if depth %}data-depth="{{ depth | urlencode }}"{% endif %}
 data-targetpath="{{ targetpath | urlencode }}"
 {% endblock %}

tests/test_gitpuller.py

@@ -64,7 +64,7 @@ class Puller(Repository):
     def __init__(self, remote, path='puller', branch="master", *args, **kwargs):
         super().__init__(path)
         remotepath = "file://%s" % os.path.abspath(remote.path)
-        self.gp = GitPuller(remotepath, branch, path, *args, **kwargs)
+        self.gp = GitPuller(remotepath, path, branch=branch, *args, **kwargs)
 
     def pull_all(self):
         for line in self.gp.pull():