Skip to content
This repository was archived by the owner on Aug 7, 2019. It is now read-only.
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 31 additions & 4 deletions lib/commands/install.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,9 @@ exports.run = function (settings, args) {
var a = argParse(args, {
'repository': {match: ['-r', '--repository'], value: true},
'target_dir': {match: ['-d', '--package-dir'], value: true},
'baseurl': {match: ['-b', '--baseurl'], value: true}
'baseurl': {match: ['-b', '--baseurl'], value: true},
'save': {match: ['-S', '--save'], value: false},
'save_dev': {match: ['-D', '--save-dev'], value: false}
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for this option, since Jam doesn't use devDependencies

});

var opt = a.options;
Expand Down Expand Up @@ -159,7 +161,6 @@ exports.reinstallPackages = function (cfg, opt, callback) {
if (err) {
return logger.error(err);
}
// TODO: write package.json if --save option provided
project.updateRequireConfig(opt.target_dir, opt.baseurl,
function (err) {
if (err) {
Expand Down Expand Up @@ -187,7 +188,6 @@ exports.installPackages = function (cfg, names, opt, callback) {
if (err) {
return callback(err);
}
// TODO: write package.json if --save option provided
project.updateRequireConfig(opt.target_dir, opt.baseurl, callback);
});
};
Expand Down Expand Up @@ -406,7 +406,34 @@ exports.installRepo = function (name, range, opt, callback) {
if (err) {
return callback(err);
}
exports.cpDir(name, v, from_cache, cdir, opt, callback);
if (opt.save || opt.save_dev) {
exports.cpDir(name, v, from_cache, cdir, opt, function() {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no error handling for the cpDir callback?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out I had no reason at all for leaving that out. Fix'd!

fs.readFile('package.json', function(err, data) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this path work when in a sub-directory of the project? Currently commands will walk up the directory hierarchy to find the project directory containing package.json.

var deps = opt.save ? 'dependencies' : 'devDependencies';
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's also possible to use jam.dependencies in package.json... devDependencies is currently unused by jam. I'd recommend we only use jam.dependencies to avoid conflicting with NPM's 'dependencies' property when saving.

...and I've just noticed you are namespacing it under 'jam' later on ;)


try {
data = JSON.parse(data.toString('utf8'));
} catch(e) {
err = e;
}
if (err) {
return callback();
}
if (!data.jam) {
data.jam = {};
}
data.jam[deps] = data.jam[deps] || {};
data.jam[deps][name] = v;

data = JSON.stringify(data, null, 2) + '\n';
fs.writeFile('package.json', data, function(err) {
callback(err);
});
});
});
} else {
exports.cpDir(name, v, from_cache, cdir, opt, callback);
}
}
);
};
Expand Down