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

Removed reliance on `stringWithUTF8String:` #1

Merged
merged 1 commit into from Nov 19, 2013

Conversation

Projects
None yet
4 participants
@hypercrypt
Copy link
Contributor

hypercrypt commented Nov 17, 2013

Hi, this looks awesome, but I am not quite sure why it is built on top of -stringWithUTF8String:.

I have modified it to remove the swizzeling. The macro syntax is the same and all unit tests still pass.

Am I missing something?

@calebd

This comment has been minimized.

Copy link

calebd commented Nov 17, 2013

Thank you! I was wondering why we needed to rely on stringWithUTF8String: at all given that everything runs through a macro anyway. This makes me feel so much better!

@dbachrach

This comment has been minimized.

Copy link
Owner

dbachrach commented Nov 17, 2013

Thanks for this. I was playing with boxed C strings as the mechanism to recognize literals and then just created the $ macro to simplify things.

[(id)@("55b") isTooLarge];

to

[$(55b) isTooLarge];

But you're right. Given that it's all going through a macro, you can just eliminate the entire boxed string implementation. I think it was a cool exploration and I hope my post on it is interesting, but this PR makes OCUDL usable in production. Will take a look and merge this in real soon.

@gvieira

This comment has been minimized.

Copy link

gvieira commented Nov 17, 2013

Hey,

Just did almost the same thing and it was about to create a PR (2142c8f). So, +1!

@hypercrypt

This comment has been minimized.

Copy link
Contributor Author

hypercrypt commented Nov 17, 2013

I had a feeling that's what happened ;) This is really cool!

@dbachrach

This comment has been minimized.

Copy link
Owner

dbachrach commented Nov 17, 2013

@gvieira Awesome. Will take a look at that too.

@dbachrach

This comment has been minimized.

Copy link
Owner

dbachrach commented Nov 19, 2013

I've updated OCUDL In Depth to address this change.

@hypercrypt

This comment has been minimized.

Copy link
Contributor Author

hypercrypt commented Nov 19, 2013

Nice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.