Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement liftoff into grunt-cli #117

Merged
merged 5 commits into from
Aug 15, 2018
Merged

Implement liftoff into grunt-cli #117

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7b8cb3f
Implement liftoff into grunt-cli
shama Feb 10, 2018
ea01dc5
Include grunt-cli version with --version
shama Feb 10, 2018
0c5f397
Fix version and help with no gruntfile
shama Mar 6, 2018
8ae6299
Fix completions to use old Grunt style for now
shama Mar 6, 2018
98ba9e6
Use ~ for deps and update v8flags to 3.0.2
shama Mar 15, 2018
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
85 changes: 55 additions & 30 deletions bin/grunt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,41 +4,66 @@

process.title = 'grunt';

// Especially badass external libs.
var findup = require('findup-sync');
var resolve = require('resolve').sync;
var Liftoff = require('liftoff');
var v8flags = require('v8flags');
var extensions = require('interpret').jsVariants;
var nopt = require('nopt');
var gruntOptions = require('grunt-known-options');
var completion = require('../lib/completion.js');
var info = require('../lib/info.js');
var pkg = require('../package.json');

// Internal libs.
var options = require('../lib/cli').options;
var completion = require('../lib/completion');
var info = require('../lib/info');
var path = require('path');
// Parse `gruntOptions` into a form that nopt can handle.
var aliases = {};
var known = {};

var basedir = process.cwd();
var gruntpath;
Object.keys(gruntOptions).forEach(function(key) {
var short = gruntOptions[key].short;
if (short) {
aliases[short] = '--' + key;
}
known[key] = gruntOptions[key].type;
});

// Parse them and return an options object.
var options = nopt(known, aliases, process.argv, 2);

// Do stuff based on CLI options.
if ('completion' in options) {
completion.print(options.completion);
Copy link
Member

Choose a reason for hiding this comment

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

Are we going to drop the completion command line flag? If someone was configuring the auto-completion as pointed in the README, they will only see Grunt trying to run their tasks directly after opening a new terminal session, which will result in an exception simular to the one in my other comment.

} else if (options.version) {
info.version();
} else if (options.gruntfile) { //Note: if both `gruntfile` and `base` are set, use `gruntfile`
basedir = path.resolve(path.dirname(options.gruntfile));
} else if (options.base) {
basedir = path.resolve(options.base);
}

try {
gruntpath = resolve('grunt', {basedir: basedir});
} catch (ex) {
gruntpath = findup('lib/grunt.js');
// No grunt install found!
if (!gruntpath) {
if (options.version) { process.exit(); }
if (options.help) { info.help(); }
info.fatal('Unable to find local grunt.', 99);
}
console.log('grunt-cli v' + pkg.version);
Copy link
Member

Choose a reason for hiding this comment

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

I think you need to exit after this line as it will cause an exception in the line 53. Here is what I got when I run grunt --version:

$ grunt --version
grunt-cli v1.2.0

assert.js:84
  throw new assert.AssertionError({
  ^
AssertionError: missing path
    at Module.require (module.js:502:3)
    at require (internal/module.js:20:19)
    at Liftoff.<anonymous> (/home/arkni/.local/lib/node_modules/grunt-cli/bin/grunt:53:17)
    at Liftoff.execute (/home/arkni/.local/lib/node_modules/grunt-cli/node_modules/liftoff/index.js:203:12)
    at module.exports (/home/arkni/.local/lib/node_modules/grunt-cli/node_modules/flagged-respawn/index.js:51:3)
    at Liftoff.<anonymous> (/home/arkni/.local/lib/node_modules/grunt-cli/node_modules/liftoff/index.js:195:5)
    at Liftoff.<anonymous> (/home/arkni/.local/lib/node_modules/grunt-cli/node_modules/liftoff/index.js:170:7)
    at _combinedTickCallback (internal/process/next_tick.js:73:7)
    at process._tickCallback (internal/process/next_tick.js:104:9)

Copy link
Member

Choose a reason for hiding this comment

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

Note that I runned that command in an arbitrary folder which doesn't have Grunt installed to its node_modules.

Copy link
Member

Choose a reason for hiding this comment

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

I think that would be fixed by doing the same as the original version of the CLI: exit if the modulePath is not defined and the user requested the version of the CLI.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sweet, fixed. Thanks for the review!

}

// Everything looks good. Require local grunt and run it.
require(gruntpath).cli();
v8flags(function (err, v8flags) {
var Grunt = new Liftoff({
name: 'grunt',
configName: 'Gruntfile',
// Support a number of languages based on file extension
extensions: extensions,
// Flags that are v8 flags will be loaded into node instead of Gruntfile
v8flags: v8flags
});
Grunt.launch({
cwd: options.cwd,
configPath: options.gruntfile,
require: options.require,
verbose: options.verbose
}, function (env) {
var tasks = options.argv.remain;
delete options.argv;
// No grunt install found!
if (!env.modulePath) {
if (options.version) {
process.exit();
}
if (options.help) {
info.help();
}
info.fatal('Unable to find local grunt.', 99);
} else {
options.gruntfile = env.configPath;
var grunt = require(env.modulePath);
grunt.tasks(tasks, options);
}
});
});
33 changes: 0 additions & 33 deletions lib/cli.js
View file
Open in desktop

This file was deleted.

2 changes: 1 addition & 1 deletion lib/info.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,6 @@ exports.helpFooter = function() {
'your project. For more information about installing and configuring grunt,',
'please see the Getting Started guide:',
'',
'http://gruntjs.com/getting-started',
'https://gruntjs.com/getting-started',
].forEach(function(str) { console.log(str); });
};
13 changes: 7 additions & 6 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@
"grunt": "bin/grunt"
},
"dependencies": {
"findup-sync": "~0.3.0",
"grunt-known-options": "~1.1.0",
"nopt": "~3.0.6",
"resolve": "~1.1.0"
"grunt-known-options": "^1.1.0",
"interpret": "^1.1.0",
"liftoff": "^2.5.0",
"nopt": "^4.0.1",
"v8flags": "^3.0.1"
Copy link
Member

Choose a reason for hiding this comment

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

are you sure you want to change from ~ to ^?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed! Thanks!

},
"devDependencies": {
"grunt": "~1.0.1",
"grunt-contrib-jshint": "~1.0.0"
"grunt": "^1.0.2",
"grunt-contrib-jshint": "^1.1.0"
},
"files": [
"bin",
Expand Down