Skip to content

Conversation

Adlai-Holler
Copy link
Contributor

No description provided.

@phausler
Copy link
Contributor

phausler commented Dec 3, 2015

actually this probably should insert instead of just indexing in the first place as well;

And a test for verifying the behavior would be useful as well:

    func test_getObjects() {
        let array : NSArray = ["foo", "bar", "baz", "foo1", "bar2", "baz3",].bridge()
        var objects = [AnyObject]()
        array.getObjects(&objects, range: NSMakeRange(1, 3))
        XCTAssertEqual(objects.count, 3)
        let fetched = [
            (objects[0] as! NSString).bridge(),
            (objects[1] as! NSString).bridge(),
            (objects[2] as! NSString).bridge(),
        ]
        XCTAssertEqual(fetched, ["bar", "baz", "foo1"])
    }

@Adlai-Holler
Copy link
Contributor Author

OK I've reimplemented it to be a little quicker, and to insert starting at 0 rather than replace. I can't get the tests to build because when I use xcrun launch-with-toolchain I get unable to find utility "launch-with-toolchain". Using xcrun 27. Confirmed the toolchain is installed and usable, so is Xcode 7.2b4. If anyone knows of a quick solution I'd like to be able to run tests ASAP. Otherwise I've gone as far as I can with this PR.

@bubski
Copy link
Contributor

bubski commented Dec 4, 2015

You need Xcode 7.2 beta installed, and use

xcode-select -switch /Applications/Xcode-beta.app/Contents/Developer

to change xcrun's tools path.
Then it should work

@Adlai-Holler
Copy link
Contributor Author

Nice @bartekchlebek! Everything's up and running. I've changed the implementation to append since that's what you do with buffers. Test is passing and I think it's ready for review!

@phausler
Copy link
Contributor

phausler commented Dec 4, 2015

Tests look good on all platforms. Overall the code looks good to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants