make commands 'context' (directory) aware

[andy-berry-dev]

Change each command so it can take an optional 'context' directory which specifies the directory being acted in. For example brjs create-blade <app> <bladeset> <blade> becomes brjs create-blade myBlade [path/to/bladeset]. We should also check whether this then allows commands to be run from any directory.

[andy-berry-dev]

Since there isn't a huge amount of acceptance criteria around this, heres my current plan:

• Allowing the final <path> argument to be optional is difficult since its ambiguous whether a user meant to leave out the argument or forgot it - so it looks like this might have to be a flagged option to prevent the ambiguity.
• Commands that are 'context aware' must be run within the context they expect. For example create-blade would expect to be run within a BladeSet, so if it was run within the apps directory it would throw an exception. This means the commands that previously took extra arguments to define what was being talked about create-blade/create-bladeset etc no longer need these arguments.
• Commands that choose not to be context aware work exactly as they did before.
• The context path is set by the current working directory and can be optionally set by using --context <some-path>. This means commands can still be run from anywhere if this flag is used.
• Commands can be optionally context aware. The AbstractContentPlugin class will define the relevant methods so any classes extending it will not need to change when the CommandPlugin API changes - these classes continue to implement doCommand(String... args).
• Any commands that are now context aware should extend ContextAwareAbstractContentPlugin and implement the method doCommand(File contextDir, Node contextNode, String... args).
• The model can do a lot of the common things like listing expected contexts in the help command output, ensuring the correct context is used when running a command etc.

Most of the groundwork for this has been done, its just the commands that now all need converting to be context aware, but I wanted to get some feedback before ploughing ahead.

@dchambers @james-shaw-turner @leggetter @thecapdan @ioanalianabalas any thoughts on this?

[leggetter]

I'd prefer the final parameter to be the optional context e.g.

cd apps/my-app/blades

# creates blade called fish in current directory
../../../sdk/ create-blade fish

# creates blade in other-app/blades directory
../../../sdk/ create-blade fish ../../../apps/other-app/blades

Seeing this highlights that the necessity to do ../../../sdk/ really isn't nice. So, I think we should make sure having brjs on the PATH makes this a nicer experience.

Longer term, we should consider doing what I think Grunt and Gulp do - have a global runner which checks to see if there is a local version of the CLI to actually use. More info available on this if required.

[andy-berry-dev]

I'd prefer the final parameter to be the optional context e.g.

Thats what I've suggested above. It can't be a straightforward argument though since the command arguments are then ambiguous. But we can make it an optional flagged option, e.g.

cd apps/my-app/blades

# creates blade called fish in current directory
../../../sdk/ create-blade fish

# creates blade in other-app/blades directory
../../../sdk/ create-blade fish --context ../../../apps/other-app/blades

Seeing this highlights that the necessity to do ../../../sdk/ really isn't nice. So, I think we should make sure having brjs on the PATH makes this a nicer experience.

Yep, agreed. For the time being, assuming only one BRJS instance is on the path, this should work in a reasonably nice way. When we look at the full global install and apps separate from the BRJS install we'll need to refine how this works.

[leggetter]

I really don't like the --context. I'm sure lots of other CLIs run in the current directory (context) unless a final path parameter is passed. Is there no way of BRJS achieving this?

[andy-berry-dev]

We can achieve it, anything is possible :smile:. The thing that makes it more difficult for us is supporting pluggable commands but trying to also augment command arguments from the outside so any model/BRJS concerns don't need to be handled by plugins.

Command plugins configure the options they support before a command is even run, and before the context is known. So for example create-blade defines its arguments as create-blade <app-name> <bladeset-name> <blade-name>, but if the command is run from an App it becomes create-blade <bladeset-name> <blade-name>, and from a BladeSet it becomes create-blade <blade-name>.

The possible ways to support an optional fifth argument might be:

• Change commands so if they are context aware commands they must define a different set of arguments for every context they want to support - this might be in the form of a map where Context -> CommandArgs. Depending on how many contexts a commands supports this might become very painful when writing command plugins.
  • As I think about it this may be a nice option as far as the code in the model goes. My main concern is how messy command plugins might become if they have to now define multiple argument types.
• Attempt the guess whether the last argument is the context directory. This might be difficult to do since we have several commands that have a file path as the last argument (build-app app-name path-to-built-app-dir) so its not as straightforward as checking if the last argument is a directory.

Theres always the option of not allowing the the context parameter and requiring commands to be run from the correct directory.

[leggetter]

Can't commands just define their last argument as greedy? http://www.martiansoftware.com/jsap/doc/ch08.html

[andy-berry-dev]

I'm not sure how that would help. We need to know whether the last arg is the context or not so so we can provide that as the context to the plugin.

[leggetter]

Couldn't we leave it up to the plugins to decide whether to use the context or not?

If they get their own final greedy argument they use that. If not, they use the context.

[dchambers]

I haven't got time to read any of these, but it seems like we should halt development till after we've had a get together with the following people: @andyberry88, @leggetter, @thecapdan, @james-shaw-turner and myself.

[dchambers]

I'll be available between 3:45 and 4:45 BTW.

[dchambers]

As things currently stand, to create an app, then create a blade within that app, then run the blade tests, I need to run the following commands:

./brjs create-app my-app
./brjs create-blade my-app default b1
./brjs test ../apps/my-app/blades/b1/

If brjs could be put on the path I could instead run these commands:

brjs create-app my-app
brjs create-blade my-app default b1
brjs test ../apps/my-app/blades/b1/

If we implemented the context-aware commands proposal I could instead run these commands:

brjs create-app my-app
cd my-app
brjs create-blade b1
brjs test blades/b1

[andy-berry-dev]

There is a slightly more in depth write up here.

I suggest we use this issue for and comments/feedback on the proposal since wiki pages don't support comments.

[janhancic]

I don't really care about the create blade etc commands, as I rarely use them. But I would love to be able to do this for running the tests:

cd apps/my-app
brjs test libs/my-lib UTs

But I shouldn't be forced to cd into the correct folder. I should be able to provide the path that overrides the current PWD. Basically, just make it work as any *nix program.