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

Rewrite most of the core_foundation module based on Foundation #88

Merged
merged 8 commits into from
Nov 30, 2017

Conversation

dgelessus
Copy link
Collaborator

@dgelessus dgelessus commented Nov 30, 2017

Implements #74.

This is a rewrite of most of the core_foundation module's functions based on Foundation classes, resulting in more readable code with no manual function declarations needed. The only things left in the core_foundation module are the basic types/constants/functions used by the async eventloop module.

The conversion functions have also been restructured a bit. from_value and to_value have been renamed to ns_from_py and py_from_ns, because the old names weren't very clear on what direction the conversion went. The at function, which previously only produced strings, is now exactly equivalent to ns_from_py. This allows things like at(42) or at([1, 2, 3]), similar to the @ prefix/operator in Objective-C. The type-specific conversion functions (to_str, to_number, etc.) have been removed, because py_from_ns can be used in place of all of them.

I don't know if the old functions from core_foundation are used much externally. I did a quick search through the Toga repo and couldn't find anything except at being used. (at has only been moved internally, so that shouldn't be an issue.) Are there any other users of Rubicon that I should check for this sort of thing? I know Rubicon isn't officially stable yet, but I still prefer not to break other people's code.

As usual, this PR includes some general cleanup and fixes, so the individual commit diffs may be easier to read than the full diff.

This happens if the contained method is an ObjCPartialMethod.
The latter gives better failure messages.
On 64-bit systems, the NS structures are exact typedefs of the CG ones.
On 32-bit, both have the same layout, but are separate declarations
with different names. This is significant in the type encodings:
on 64-bit, both NSPoint and CGPoint encode as {CGPoint=dd}, but on
32-bit, NSPoint encodes as {_NSPoint=ff}, and CGPoint as {CGPoint=ff}.
Copy link
Member

@freakboy3742 freakboy3742 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why I bother doing a careful line-by-line teardown of your patches... they're always awesome, and I almost never find anything to fault.

The only thing that gave me pause was the way NSData conversion was being handled (line 1729 of runtime.py) - the name string_at() looked off on first inspection. However, a quick check of the ctype docs shows it's referring to a C string - which isn't very helpful from a Python naming POV, but is otherwise entirely accurate. A quick comment to explain the odd naming might be helpful; but otherwise - 👏 Once again, an amazing contribution.

@dgelessus
Copy link
Collaborator Author

Thanks :) I appreciate your reviews, it's always good to have someone else look for logic errors and such, and to get feedback on the API.

Yeah, the naming of string_at is unfortunate. I think it's a leftover from Python 2, where str was 8-bit, so it was the natural counterpart to C char strings. The Unicode version is wstring_at, which converts C wchar_t strings.

@dgelessus dgelessus merged commit e785067 into beeware:master Nov 30, 2017
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.

2 participants