Skip to content
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

Use Correct Source Index in NSArray.getObjects #7

Merged
merged 4 commits into from Dec 4, 2015

Conversation

Adlai-Holler
Copy link
Contributor

No description provided.

@phausler
Copy link
Member

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
Member

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.

None yet

3 participants