Feature: Addons add commands to the CLI #1196
Conversation
| @@ -16,6 +17,11 @@ module.exports = Command.extend({ | |||
| // Display usage for all commands. | |||
| this.ui.write('Available commands in ember-cli:\n'); | |||
|
|
|||
| if(this.project.initializeAddons && this.project.addonCommands) { | |||
There was a problem hiding this comment.
When would either of these be false?
There was a problem hiding this comment.
tests.. How would you recommend resolving this? I've considered created a simple project object fixture to use throughout the tests rather than it being defined all over the place manually
There was a problem hiding this comment.
I thought we used https://github.com/stefanpenner/ember-cli/blob/master/tests/helpers/mock-project.js throughout. Are there cases where adding it there wouldn't work?
There was a problem hiding this comment.
OK, gotcha. This is OK with me then. I would generally like to fix those other tests so we can avoid this check, but that is probably the subject for another PR.
|
Working on fixing the tests real quick |
|
|
||
| // Attempt to find command in ember-cli core commands | ||
| var command = findCommand(commands, commandName); | ||
| if(command) { |
There was a problem hiding this comment.
whitespace between if (
|
we should consider:
this is a great step forward, i am quite excited:) |
|
@stefanpenner Currently it isn't letting you override a core command in execution, though it will be listed twice in the help. I can make it throw an error instead though if you'd prefer For number 2, you can namespace it if you want right now, it's just not required. If we want to make it required I can do that as well. Are you just thinking we should to distinguish addon commands vs core? |
|
@jakecraige rather then throwing, lets print out and error.. as for name spacing, I am also curious about this. If we force namspacing, it will prevent collisions but be more verbose. Thoughts? |
|
I would personally prefer to go without namespacing initially. We can always revisit if we see folks abusing the system, but if we require namespacing that inherently means that commands become quite a bit longer to type out. |
|
@stefanpenner I don't see much of a benefit right now since I've now got it logging a warning to annoy you if you do override it and it won't actually let you anyways. If it becomes an issue we could easily swap it to print out an error instead |
|
@jakecraige sound ok |
|
@stefanpenner and @rjackson This seems good now. Let me know if there's anything else we need for it |
|
|
||
| addonPkg['ember-addon'] = addonPkg['ember-addon'] || {}; | ||
| if(fs.existsSync(pkgPath)) { |
There was a problem hiding this comment.
The generator tests freak out without it, it tries to require the file and npm install hasn't been ran so it throws an error
|
Will need a rebase (likely due to the changelog changing). |
|
@rjackson done |
|
rebase is needed again (Sorry) |
…Commands when it's done
|
@stefanpenner done |
|
LGTM |
|
👍 |
Feature: Addons add commands to the CLI
|
You guys are awesome!!! |
You can now have your addon implement a
includedCommandshook which returns an object that will be merged with ember-cli's default commands and passed into the Command.extend method for you.