Skip to content

feat(format): add code formatting #200

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.

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

Merged
merged 1 commit into from
Feb 17, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
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
18 changes: 13 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -176,13 +176,21 @@ ng github-pages:deploy

Checkout [angular-cli-github-pages addon](https://github.com/IgorMinar/angular-cli-github-pages) docs for more info.

### Linting code
### Linting and formatting code

You can lint your app code by running `ng lint`.
This will use the `lint` npm script that in generated projects uses `tslint`.
You can lint or format your app code by running `ng lint` or `ng format` respectively.
This will use the `lint`/`format` npm script that in generated projects uses `tslint`/`clang-format`.

You can modify the `lint` script in `package.json` to run whatever linting tool
you prefer and `ng lint` will still run it.
You can modify the these scripts in `package.json` to run whatever tool you prefer.


### Formatting code

You can format your app code by running `ng format`.
This will use the `format` npm script that in generated projects uses `clang-format`.

You can modify the `format` script in `package.json` to run whatever formatting tool
you prefer and `ng format` will still run it.


## Known issues
Expand Down
3 changes: 3 additions & 0 deletions addon/ng2/blueprints/ng2/files/.clang-format
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Language: JavaScript
BasedOnStyle: Google
ColumnLimit: 100
4 changes: 3 additions & 1 deletion addon/ng2/blueprints/ng2/files/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@
"scripts": {
"start": "ng server",
"postinstall": "typings install --ambient",
"lint": "tslint src/**/*.ts"
"lint": "tslint src/**/*.ts",
"format": "clang-format -i -style=file --glob=src/**/*.ts"
},
"private": true,
"dependencies": {
"angular2": "2.0.0-beta.6",
"clang-format": "^1.0.35",
"es6-promise": "^3.0.2",
"es6-shim": "^0.33.3",
"reflect-metadata": "0.1.2",
Expand Down
20 changes: 20 additions & 0 deletions addon/ng2/commands/format.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/* jshint node: true */
'use strict';

var Command = require('ember-cli/lib/models/command');
var FormatTask = require('../tasks/format');

module.exports = Command.extend({
name: 'format',
description: 'Formats code in existing project',
works: 'insideProject',
run: function() {
var formatTask = new FormatTask({
ui: this.ui,
analytics: this.analytics,
project: this.project
});

return formatTask.run();
}
});
3 changes: 2 additions & 1 deletion addon/ng2/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ module.exports = {
'install' : require('./commands/install'),
'uninstall' : require('./commands/uninstall'),
'test' : require('./commands/test'),
'lint' : require('./commands/lint')
'lint' : require('./commands/lint'),
'format' : require('./commands/format')
};
}
};
23 changes: 23 additions & 0 deletions addon/ng2/tasks/format.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/* jshint node: true */
'use strict';

var Promise = require('ember-cli/lib/ext/promise');
var Task = require('ember-cli/lib/models/task');
var exec = Promise.denodeify(require('child_process').exec);
Copy link
Member

Choose a reason for hiding this comment

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

OOC, couldn't Promise and exec be imported when the task is run (so that addon/ng2/index.js "loads" faster) ?
(I guess they are already imported elsewhere, so it won't make any real difference, but wouldn't it theoretically be better ?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suppose, in the case where they are first imported here, it would be faster, yes.

Leaving them at the top seems to be the normal usage pattern, and it's also a bit more forwards compatible with the ES6 mindset. I'm also not a big fan of conditional imports myself, they're sometimes hard to find, test and debug.

Come to think of it, I remember reading they opted with toplevel imports for a number of reasons that were meant to get around problems with node's modules system.

I don't see much advantage in this case to only requiring them when the task is run.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, it was more of a theoretical question, because they are surely already imported elsewhere.
Thx for the response.

I'm also not a big fan of conditional imports myself

Interesting (considering how chalk is imported below)... 😛

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes - guilty as charged. I copy pasted an existing task and took it down to it's bare minimum. The chalk require was there as is, and me not being fully knowledgeable of ember-cli, simply left it.


module.exports = Task.extend({
run: function() {
var chalk = require('chalk');
var ui = this.ui;

return exec('npm run format')
.then(function() {
ui.writeLine(chalk.green('Successfully formatted files.'));
})
.catch(function( /*error*/ ) {
ui.writeLine(chalk.red(
'Couldn\'t do \'npm run format\'. Please check this script exists in your package.json.'
));
});
}
});