Skip to content

Commit

Permalink
Using ES6 features
Browse files Browse the repository at this point in the history
  • Loading branch information
Jaydson committed Dec 9, 2014
1 parent 0197de5 commit 7979276
Show file tree
Hide file tree
Showing 7 changed files with 234 additions and 226 deletions.
13 changes: 7 additions & 6 deletions src/bin/cli/logo.js
View file
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
var localconfig = require('../config'),
helpers = require('../helpers'),
clc = helpers.cliColor(),
logo = '\n' +
import { version } from '../config';
import { cliColor } from '../helpers';

var clc = cliColor(),
logo = clc.message('\n' +
'|_| _ _ _ _ _ _ . _ \n' +
'| |(_|| | | |(_)| ||(_ \n' +
' ' + localconfig.version + ' \n';
' ' + version + ' \n');

module.exports = clc.message(logo);
export default logo;
44 changes: 21 additions & 23 deletions src/bin/cli/program.js
View file
Original file line number Diff line number Diff line change
@@ -1,69 +1,67 @@
require('grunt-6to5/node_modules/6to5/polyfill');
var program = require('commander');

var localconfig = require('../config'),
helpers = require('../helpers'),
program = require('commander'),
logo = require('../cli/logo');
import { version } from '../config';
import { cliColor } from '../helpers';
import logo from './logo';
import { init, config, newFile, run } from './util';

program
.version(localconfig.version);
.version(version);

program
.command('init')
.description('Init your static website')
.action(function(path) {
var util = require('../cli/util');
.action((path) => {
console.log(logo);
util.init(typeof path === 'string' ? path : './');
init(typeof path === 'string' ? path : './');
});

program
.command('config')
.description('Config your static website')
.action(function() {
var util = require('../cli/util');
.action(() => {
console.log(logo);
util.config();
config();
});

program
.command('build')
.description('Build your static website')
.action(function() {
var core = require('../core');
.action(() => {
let core = require('../core');
core.init();
});

program
.command('new_post ["title"]')
.description('Create a new post')
.action(function(title) {
require('../cli/util').newFile('post', title);
.action((title) => {
newFile('post', title);
});

program
.command('new_page ["title"]')
.description('Create a new page')
.action(function(title) {
require('../cli/util').newFile('page', title);
.action((title) => {
newFile('page', title);
});

program
.command('run [port]')
.description('Run you static site locally. Port is optional')
.action(function(port = 9356) {
var util = require('../cli/util'),
core = require('../core'),
.action(function(port = 9356) { // We're not using arrow function here due to an jshint issue
let core = require('../core'),
build = core.init();
if (build) {
build.then(function() {
util.run(port);
run(port);
});
}
});

program.on('*', function(args) {
var clc = helpers.cliColor();
program.on('*', (args) => {
let clc = cliColor();
console.error('Unknown command: ' + clc.error(args[0]));
process.exit(1);
});
Expand Down
Loading

7 comments on commit 7979276

@jaydson
Copy link
Contributor

@jaydson jaydson commented on 7979276 Dec 9, 2014

Choose a reason for hiding this comment

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

//cc @UltCombo
please make a review when you're able.

@UltCombo
Copy link
Member

Choose a reason for hiding this comment

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

LGTM, nice work! 👍

Though, I'm not sure if it is a good idea to replace all regular functions with arrow functions. We must be wary that arrow functions implicitly return the value of the last expression, and this may bring unexpected side-effects.

Also, our JSCS rules only allow a single var keyword per scope (requireMultipleVarDecl: "onevar"), while this commit does the complete opposite with let (as if we had disallowMultipleVarDecl for let). We should make this consistent, either both var and let declarations have to be joined or should never be joined together.

@jaydson
Copy link
Contributor

Choose a reason for hiding this comment

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

arrow functions implicitly return the value of the last expression

I don't think this is true.
When we have a function body we must to explicitly return the value we want.
Example:

let foo = x => {
  return 'foo' + x;
}

Arrow function have implicitly return when we don't have a function body:

let foo = x => 'foo' + x.

Please correct me if i missed something.

@jaydson
Copy link
Contributor

Choose a reason for hiding this comment

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

About let and var what you suggest?
I totally agree we need consistency in our code style.

@UltCombo
Copy link
Member

Choose a reason for hiding this comment

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

When we have a function body we must to explicitly return the value we want.

You're completely right, my bad. A quick test on Nightly console shows you're right:

<<< (() => 1)()
>>> 1
<<< (() => {1})()
>>> undefined

I still confuse ES6 arrow functions with CoffeScript's fat arrows, which do implicit return the last expression's value when there is no explicit return as the last statement of the function. (e.g. demo). Thankfully ES6 is saner than that. 😉

@jaydson
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@UltCombo
Copy link
Member

Choose a reason for hiding this comment

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

About let and var what you suggest?

Well, these code style decisions are mostly personal opinion, so I'm not sure. The way you wrote looks fine to me, and that also follows Google's code style guide (no multiple declaration).

Please sign in to comment.