JohnSundell / ShellOut

Easily run shell commands from a Swift script or command line tool
MIT License
870 stars 83 forks source link

An asynchronous version of call to shellOut #34

Open ghost opened 6 years ago

ghost commented 6 years ago

Hi John,

Asynchronous calls looks like this.

shellOut(to: "echo", arguments: ["Hello"]) { completion in

    do {
        let output = try completion()
        XCTAssertEqual(output, "Hello")
    } catch {
        XCTFail()
    }

 }      

There is a discussion here about this completion block syntax.

ShellOut.swift now only contains the public interface

Created the following for internal logic

ShellOutError.swift ShellOutCommand.swift String+Extensions.swift Data+Extensions.swift Process+Extensions.swift

ShellOutTests.swift MARK: // Asynchronous and MARK: // Synchronous

Original tests pushed to bottom

Looking forward to hearing your thoughts.

iainsmith commented 6 years ago

Hey @rob-nash I've proposed that we drop the FileHandle API for async options in #33.

Regarding the implementation, I imagine it would be preferable to have a single async version of launchBash(...) that we can wrap in a semaphore to use in synchronous apis

Is there a reason why we need the duplication that I'm missing?

ghost commented 6 years ago

Hi @iainsmith

I was hoping to hear from John about FileHandle parameters and API style before I continued the work.

It has been a while. So I'm going to leave all of the original synchronous API in place and build the asynchronous code as an extension to the public API interface. There will be some minor duplication, as a consequence.

The sync code uses FileHandle params and the async code does not.

Travis is complaining about async tests at the moment. Those tests pass locally for me. Travis doesn't seem to recognise the 'expectation' API in XCTest framework. Sigh.

JohnSundell commented 6 years ago

@rob-nash @iainsmith Hey guys. Just wanted to quickly check-in here and say that I've seen these discussions and I'm planning to look into the various ideas and implementations shortly, to give you my feedback. At the moment I'm a bit busy with other projects, but I really appreciate your efforts in making ShellOut better, so hoping to merge one of these implementations in as soon as possible 👍

ghost commented 6 years ago

ShellOutTests+Linux.swift Is missing some tests. I'm not sure if this was intentional?

ghost commented 6 years ago

travis.yml changes as per suggestions here