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

Proposed by Marcus Tomlinson on 2016-03-03
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) 2016-03-03 Approve on 2016-03-03
PS Jenkins bot (community) continuous-integration Needs Fixing on 2016-03-03
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 on 2016-03-03

Cleanup

139. By Marcus Tomlinson on 2016-03-03

Added comment

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 on 2016-03-04

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
1=== modified file 'src/tool/main.cc'
2--- src/tool/main.cc 2016-01-15 10:19:11 +0000
3+++ src/tool/main.cc 2016-03-04 05:13:05 +0000
4@@ -196,7 +196,17 @@
5 if (boost::algorithm::ends_with(std::string(argv[1]), "install") && argc > 3
6 && std::string(argv[3]) != "unity-js-scopes") // Install an npm module
7 {
8- std::string npm_module = argv[3];
9+ std::string npm_module_raw = argv[3];
10+ std::string npm_module = npm_module_raw;
11+ std::string npm_module_full = npm_module_raw;
12+
13+ // Extract npm_module from npm_module_raw by chopping off the "@<version>" postfix (if present)
14+ auto pos = npm_module_raw.find('@');
15+ if (pos != std::string::npos)
16+ {
17+ npm_module = npm_module_raw.substr(0, pos);
18+ npm_module_full = npm_module + " (" + npm_module_raw.substr(pos+1) + ")";
19+ }
20
21 // Don't reinstall an already installed module unless "reinstall" is called
22 if (std::string(argv[1]) == "reinstall" || !boost::filesystem::exists(modules_dir + "/" + npm_module))
23@@ -208,9 +218,9 @@
24 }
25
26 // Install the npm module
27- std::cout << "Installing npm module '" << npm_module << "' to '" << modules_dir << "' ..." << std::endl;
28+ std::cout << "Installing npm module '" << npm_module_full << "' to '" << modules_dir << "' ..." << std::endl;
29
30- std::string node_cmd = "node /node_modules/npm/cli.js --prefix='" + modules_dir + "/../' --ignore-scripts install " + npm_module;
31+ std::string node_cmd = "node /node_modules/npm/cli.js --prefix='" + modules_dir + "/../' --ignore-scripts install " + npm_module_raw;
32 std::cout << "Running '" << node_cmd << "' ..." << std::endl;
33 result = system(node_cmd.c_str());
34 }

Subscribers

People subscribed via source and target branches

to all changes: