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 {
