-
Notifications
You must be signed in to change notification settings - Fork 85
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
Simplify Task command API #29
Comments
The Task api is written the way it is for flexibility, the requirements of NSTask itself, and the restrictions of JavaScriptCore. While it might be nice to have a more consolidated one line command, it's not possible or practical. Separating the arguments/calls the way they are, allows for flexibility in not only the commands that can be run, but how arguments are passed to those commands. Bundling the command call and command arguments together won't work because NSTask requires the arguments to be passed as an NSArray. So that means before the arguments are passed to NSTask, they'd have to be parsed and split up into an NSArray. This would be fine if every command accepts the same type, format, and position of arguments, but they don't. So there's no way to reliably parse the arguments for every possible task. Grouping callbacks together in a single object, or in a config object won't work either because of the limitations of JavaScriptCore. JS Objects containing functions don't get converted properly and the function is lost upon conversion. |
So would there be a problem with something like this? var myTask = MacGap.Task.launch('/bin/ls', ['-la', '/tmp'], myTaskCompletedCallback); |
That is actually nearly identical to http://nodejs.org/api/child_process.html#child_process_child_process_execfile_file_args_options_callback |
As @perifer pointed out in #28 the Task command could benefit from simplification.
At present the JS API reflects the Objective-C mindset where you create a new NSTask instance, then call methods on it to configure it.
A more JavaScript-y API (using jQuery as a model) would probably be:
I propose something like:
or else just the one callback:
I'm happy to do the refactoring.
The text was updated successfully, but these errors were encountered: