Refactor fetch to use an options object instead of many parameters

Project: http://git-wip-us.apache.org/repos/asf/cordova-plugman/repo
Commit: http://git-wip-us.apache.org/repos/asf/cordova-plugman/commit/a554dfe9
Tree: http://git-wip-us.apache.org/repos/asf/cordova-plugman/tree/a554dfe9
Diff: http://git-wip-us.apache.org/repos/asf/cordova-plugman/diff/a554dfe9

Branch: refs/heads/master
Commit: a554dfe920eb28b9ac6e1396b02e2efb7f696127
Parents: 151b854
Author: Braden Shepherdson <[email protected]>
Authored: Tue May 28 14:58:46 2013 -0400
Committer: Braden Shepherdson <[email protected]>
Committed: Tue May 28 14:58:46 2013 -0400

----------------------------------------------------------------------
 spec/fetch.spec.js |   10 +++++-----
 src/fetch.js       |   14 ++++++++------
 src/install.js     |    2 +-
 3 files changed, 14 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cordova-plugman/blob/a554dfe9/spec/fetch.spec.js
----------------------------------------------------------------------
diff --git a/spec/fetch.spec.js b/spec/fetch.spec.js
index 79afaa4..c5f747c 100644
--- a/spec/fetch.spec.js
+++ b/spec/fetch.spec.js
@@ -19,7 +19,7 @@ describe('fetch', function() {
 
     describe('local plugins', function() {
         it('should copy locally-available plugin to plugins directory', 
function() {
-            fetch(test_plugin, temp, false);
+            fetch(test_plugin, temp);
             expect(fs.existsSync(copied_plugin_path)).toBe(true);
         });
        // it('should copy locally-available plugin to plugins directory when 
specified with a trailing slash', function() {
@@ -27,21 +27,21 @@ describe('fetch', function() {
        //     expect(fs.existsSync(copied_plugin_path)).toBe(true);
        // });
         it('should create a symlink if used with `link` param', function() {
-            fetch(test_plugin, temp, true);
+            fetch(test_plugin, temp, { link: true });
             
expect(fs.lstatSync(copied_plugin_path).isSymbolicLink()).toBe(true);
         });
     });
     describe('remote plugins', function() {
         it('should call clonePluginGitRepo for https:// and git:// based 
urls', function() {
             var s = spyOn(plugins, 'clonePluginGitRepo');
-            fetch("https://github.com/bobeast/GAPlugin.git";, temp, false);
+            fetch("https://github.com/bobeast/GAPlugin.git";, temp);
             expect(s).toHaveBeenCalled();
         });
         it('should call clonePluginGitRepo with subdir if applicable', 
function() {
             var s = spyOn(plugins, 'clonePluginGitRepo');
             var url = "https://github.com/bobeast/GAPlugin.git";;
             var dir = 'fakeSubDir';
-            fetch(url, temp, false, dir);
+            fetch(url, temp, { subdir: dir });
             expect(s).toHaveBeenCalledWith(url, temp, dir, undefined, 
undefined);
         });
         it('should call clonePluginGitRepo with subdir and git ref if 
applicable', function() {
@@ -49,7 +49,7 @@ describe('fetch', function() {
             var url = "https://github.com/bobeast/GAPlugin.git";;
             var dir = 'fakeSubDir';
             var ref = 'fakeGitRef';
-            fetch(url, temp, false, dir, ref);
+            fetch(url, temp, { subdir: dir, git_ref: ref });
             expect(s).toHaveBeenCalledWith(url, temp, dir, ref, undefined);
         });
         it('should throw if used with url and `link` param', function() {

http://git-wip-us.apache.org/repos/asf/cordova-plugman/blob/a554dfe9/src/fetch.js
----------------------------------------------------------------------
diff --git a/src/fetch.js b/src/fetch.js
index e58ef00..201f495 100644
--- a/src/fetch.js
+++ b/src/fetch.js
@@ -4,20 +4,22 @@ var shell   = require('shelljs'),
     xml_helpers = require('./util/xml-helpers'),
     path    = require('path');
 
-module.exports = function fetchPlugin(plugin_dir, plugins_dir, link, subdir, 
git_ref, callback) {
+// possible options: link, subdir, git_ref
+module.exports = function fetchPlugin(plugin_dir, plugins_dir, options, 
callback) {
     // Ensure the containing directory exists.
     shell.mkdir('-p', plugins_dir);
 
-    subdir = subdir || '.';
+    options = options || {};
+    options.subdir = options.subdir || '.';
 
     // clone from git repository
     if(plugin_dir.indexOf('https://') == 0 || plugin_dir.indexOf('git://') == 
0) {
-        if (link) {
+        if (options.link) {
             var err = new Error('--link is not supported for git URLs');
             if (callback) callback(err);
             else throw err;
         } else {
-            plugins.clonePluginGitRepo(plugin_dir, plugins_dir, subdir, 
git_ref, callback);
+            plugins.clonePluginGitRepo(plugin_dir, plugins_dir, 
options.subdir, options.git_ref, callback);
         }
     } else {
         if (plugin_dir.lastIndexOf('file://', 0) === 0) {
@@ -26,14 +28,14 @@ module.exports = function fetchPlugin(plugin_dir, 
plugins_dir, link, subdir, git
 
         // Copy from the local filesystem.
         // First, read the plugin.xml and grab the ID.
-        plugin_dir = path.join(plugin_dir, subdir);
+        plugin_dir = path.join(plugin_dir, options.subdir);
         var xml = xml_helpers.parseElementtreeSync(path.join(plugin_dir, 
'plugin.xml'));
         var plugin_id = xml.getroot().attrib.id;
 
         var dest = path.join(plugins_dir, plugin_id);
 
         shell.rm('-rf', dest);
-        if (link) {
+        if (options.link) {
             fs.symlinkSync(plugin_dir, dest, 'dir');
         } else {
             shell.mkdir('-p', dest);

http://git-wip-us.apache.org/repos/asf/cordova-plugman/blob/a554dfe9/src/install.js
----------------------------------------------------------------------
diff --git a/src/install.js b/src/install.js
index dfb331d..f744468 100644
--- a/src/install.js
+++ b/src/install.js
@@ -32,7 +32,7 @@ function possiblyFetch(actions, platform, project_dir, id, 
plugins_dir, options,
     if (!fs.existsSync(plugin_dir)) {
         // if plugin doesnt exist, use fetch to get it.
         // TODO: Actual value for git_ref.
-        require('../plugman').fetch(id, plugins_dir, false, '.', 
options.git_ref, function(err, plugin_dir) {
+        require('../plugman').fetch(id, plugins_dir, { link: false, subdir: 
'.', git_ref: options.git_ref }, function(err, plugin_dir) {
             if (err) {
                 callback(err);
             } else {

Reply via email to