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.

Sign up for GitHub

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

Jump to bottom

Use objc2 and its framework crates #74

Merged
merged 1 commit into from
Apr 30, 2024
Merged

Use objc2 and its framework crates #74

merged 1 commit into from
Apr 30, 2024

Conversation

madsmtm
Copy link
Contributor

@madsmtm madsmtm commented Apr 30, 2024

This catches a few extra error cases related to paths, as well as making it easier to ensure that memory management rules are upheld.

Concretely, it fixes a leak in the passing of the NSArray to writeObjects.

@madsmtm
Use objc2 and its framework crates
0c2ace5 
This catches a few extra error cases related to paths, as well as making
it easier to ensure that memory management rules are upheld.

Concretely, it fixes a leak in the passing of the `NSArray` to
`writeObjects`.
@madsmtm madsmtm mentioned this pull request Apr 30, 2024
Open
65 tasks
chrisduerr
chrisduerr requested changes Apr 30, 2024
View reviewed changes
Copy link
Member

@chrisduerr chrisduerr left a comment

Choose a reason for hiding this comment

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

What happened to objc, is it not communicative? Seems like you're the maintainer of the objc2 crates, is that correct?

src/osx_clipboard.rs Show resolved Hide resolved
@madsmtm
Copy link
Contributor Author

madsmtm commented Apr 30, 2024

What happened to objc, is it not communicative? Seems like you're the maintainer of the objc2 crates, is that correct?

Apologies, should've given a more detailed explanation, assumed you knew it already. I'm the maintainer of objc2, which we've used for a while in Winit. The fork is a few years old now, the reasoning behind it can be found in SSheldon/rust-objc#101. Nowadays, objc2 contains a lot more features, including the msg_send_id! macro described above that ensures we follow memory management rules correctly.

Did that give a bit more insight? Feel free to ask about anything else that's unclear!

@nixpulvis
Copy link

Doing the same things as winit makes a lot of sense since alacirtty is a consumer of both this crate and winit.

@chrisduerr
Copy link
Member

the reasoning behind it can be found in SSheldon/rust-objc#101

Sounds like you just got ignored lol.

chrisduerr
chrisduerr approved these changes Apr 30, 2024
View reviewed changes
Copy link
Member

@chrisduerr chrisduerr left a comment

Choose a reason for hiding this comment

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

@madsmtm I'd assume you've tested this against some client? Or used the examples to confirm it actually works?

I have no way to test macOS myself, so I'll hold off merging until I can get a confirmation on this.

@nixpulvis
Copy link

nixpulvis commented Apr 30, 2024 •
edited
Loading

The hello_world example seems to work on macOS. I'd be happy to test more thoroughly, but I'm not sure exactly how to trigger the NULLs this PR is primarily concerned with.

Happy to add some tests in the process if given guidance.

@chrisduerr
Copy link
Member

I'm not sure exactly how to trigger the NULLs this PR is primarily concerned with.

Shouldn't really be possible, so doesn't need testing. Thanks for the help.

@chrisduerr chrisduerr merged commit 7a0c23c into alacritty:master Apr 30, 2024
4 checks passed
@madsmtm madsmtm deleted the objc2 branch April 30, 2024 19:00
madsmtm added a commit to madsmtm/crossfont that referenced this pull request May 21, 2024
@madsmtm
Use objc2 for determining whether font smoothing is enabled
c9c1430 
`objc2` / `objc2-foundation` is much more type-safe than `cocoa`, and
automatically ensures that memory management-rules are upheld.

See also alacritty/copypasta#74.
@madsmtm madsmtm mentioned this pull request May 21, 2024
Merged
chrisduerr pushed a commit to alacritty/crossfont that referenced this pull request May 22, 2024
@madsmtm
Use objc2 for macOS font smoothing determination
895ccc1 
`objc2` / `objc2-foundation` is much more type-safe than `cocoa`, and
automatically ensures that memory management-rules are upheld.

See also alacritty/copypasta#74.
@madsmtm madsmtm mentioned this pull request Nov 18, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chrisduerr chrisduerr chrisduerr approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@madsmtm @nixpulvis @chrisduerr