Closed ghost closed 6 years ago
Hi @rob-nash, and thanks for your contribution. However, this change is too big to be considered at the moment, since we're not looking to make such big architectural changes. The focus for Marathon right now is to keep up with changes in Xcode and the underlying developer tools, to fix bugs, and to implement missing features that would make using it easier. After that, we can take a look at improving the architecture, but it would have to be done piece by piece, not with such a big change. Thanks anyway π
I understand @JohnSundell no worries π
LOVE YOUR CODE BRO MY EYES BLEED WITH HAPPINESS
I have enjoyed spending time on Marathon and I hope you like this pull request.
I noticed Marathon is using the command pattern so I started reading up about it. Then I started looking at Marathon and I noticed a few potential bugs or risky areas that I might be able to improve.
No hard feelings if you don't want to pull this one in.
Tasks
If the
ScriptManager
throws during the early setup phase, the errorCouldNotPerformSetup
is thrown for all tasks. However, the following tasks do not require theScriptManager
.Further, the following tasks only use a
PackageManager
.And the
Help
task uses neither.The following do not consume any options or supplementary arguments.
So have fixed these issues in this pull request by reworking tasks so that they are not all treated the same.
A Collection Of Strings
The input at the CommandLine includes
These appear in marathon as an array of strings and are rarely unpacked into types.
So for instance...
Could be represented like this
This pull request implements this by digesting the array of command line arguments (strings) where possible (as early as possible).
The
RunTask
task, which runs a script that may itself require arguments, is represented in the same way i.e. with properties.The benefit of this approach is that the task is only given the arguments relevant for the task at hand. All other arguments are stripped away.
Building
Arguments no longer passed in as strings here.
Printer
I made this change quite early on. I'm not sure why. Would you like me to change it back? π
Before
After