Merge lp:~marcustomlinson/unity-js-scopes/lp-1552661 into lp:unity-js-scopes

Proposed by Marcus Tomlinson
Status: Merged
Merged at revision: 137
Proposed branch: lp:~marcustomlinson/unity-js-scopes/lp-1552661
Merge into: lp:unity-js-scopes
Diff against target: 34 lines (+13/-3)
1 file modified
src/tool/main.cc (+13/-3)
To merge this branch: bzr merge lp:~marcustomlinson/unity-js-scopes/lp-1552661
Reviewer Review Type Date Requested Status
Alexandre Abreu (community) Approve
PS Jenkins bot (community) continuous-integration Needs Fixing
Review via email: mp+287913@code.launchpad.net

Commit message

Support for "unity-js-scopes-tool install <module>@<version>"

Description of the change

Support for "unity-js-scopes-tool install <module>@<version>"

Although we do inadvertently support this command, our check for whether the module is already installed breaks due to a directory name mismatch.

To post a comment you must log in.
138. By Marcus Tomlinson

Cleanup

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
139. By Marcus Tomlinson

Added comment

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Alexandre Abreu (abreu-alexandre) wrote :

Just a nit suggestion, it might be good in the "Installing npm module" std::cout line to also mention the version being installed,

otherwise ok,

review: Approve
140. By Marcus Tomlinson

Mention the version being installed in the "Installing npm module..." output

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/tool/main.cc'
--- src/tool/main.cc 2016-01-15 10:19:11 +0000
+++ src/tool/main.cc 2016-03-04 05:13:05 +0000
@@ -196,7 +196,17 @@
196 if (boost::algorithm::ends_with(std::string(argv[1]), "install") && argc > 3196 if (boost::algorithm::ends_with(std::string(argv[1]), "install") && argc > 3
197 && std::string(argv[3]) != "unity-js-scopes") // Install an npm module197 && std::string(argv[3]) != "unity-js-scopes") // Install an npm module
198 {198 {
199 std::string npm_module = argv[3];199 std::string npm_module_raw = argv[3];
200 std::string npm_module = npm_module_raw;
201 std::string npm_module_full = npm_module_raw;
202
203 // Extract npm_module from npm_module_raw by chopping off the "@<version>" postfix (if present)
204 auto pos = npm_module_raw.find('@');
205 if (pos != std::string::npos)
206 {
207 npm_module = npm_module_raw.substr(0, pos);
208 npm_module_full = npm_module + " (" + npm_module_raw.substr(pos+1) + ")";
209 }
200210
201 // Don't reinstall an already installed module unless "reinstall" is called211 // Don't reinstall an already installed module unless "reinstall" is called
202 if (std::string(argv[1]) == "reinstall" || !boost::filesystem::exists(modules_dir + "/" + npm_module))212 if (std::string(argv[1]) == "reinstall" || !boost::filesystem::exists(modules_dir + "/" + npm_module))
@@ -208,9 +218,9 @@
208 }218 }
209219
210 // Install the npm module220 // Install the npm module
211 std::cout << "Installing npm module '" << npm_module << "' to '" << modules_dir << "' ..." << std::endl;221 std::cout << "Installing npm module '" << npm_module_full << "' to '" << modules_dir << "' ..." << std::endl;
212222
213 std::string node_cmd = "node /node_modules/npm/cli.js --prefix='" + modules_dir + "/../' --ignore-scripts install " + npm_module;223 std::string node_cmd = "node /node_modules/npm/cli.js --prefix='" + modules_dir + "/../' --ignore-scripts install " + npm_module_raw;
214 std::cout << "Running '" << node_cmd << "' ..." << std::endl;224 std::cout << "Running '" << node_cmd << "' ..." << std::endl;
215 result = system(node_cmd.c_str());225 result = system(node_cmd.c_str());
216 }226 }

Subscribers

People subscribed via source and target branches

to all changes: