dojo / cli

:rocket: Dojo - command line tooling.
http://dojo.io
Other
26 stars 34 forks source link

Refactor to separate out concerns of 'tty processing', 'loading commands' and 'running commands' #73

Closed rishson closed 5 years ago

rishson commented 7 years ago

I think it would improve testability if we separated the code into:

1) tty processing using yargs 2) installed commands resolution and loading 3) running commands (in-built and installed)

Commands should not really be aware of their invocation context, i.e. that they are being run in a terminal. This would make commands easier to test. At the moment we have: a command loader implementation (command) that knows about screen width. a command loader (loadCommands) that knows about yargs a command runner (registerCommands) that gets passed a command loader implementation for non-loader functionality.

I propose the following refactor:

command -> commandLoader loadCommands -> runCommandLoader registerCommands -> runCommands

where

index.ts is the only tty aware module commandLoader is the only file system aware module runCommandLoader becomes a simple loader executor runCommands becomes a simple command executor in-built commands become async commands in a /commands subdir and simply get added to the list of commands to run. command specific text is kept with the command, rather than collected up into a cross-command text.ts file. split commnds/version into the Command and the outdated check.

In addition, unit tests contain a test for index.ts. This is actually a functional test and should be moved into a function test dir, and should be tested against without mocks.

I think this is a cleaner separation of concerns that should be easier to test and extend.

kitsonk commented 7 years ago

References #140