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

Improve NSAttributedString ctors in foundation.cs #14489

Closed
chamons opened this issue Mar 24, 2022 · 2 comments · Fixed by #21727
Closed

Improve NSAttributedString ctors in foundation.cs #14489

chamons opened this issue Mar 24, 2022 · 2 comments · Fixed by #21727
Labels
enhancement The issue or pull request is an enhancement
Milestone
Future

Comments

@chamons
Copy link
Contributor

chamons commented Mar 24, 2022

During the great breaking change API scrub, one API was missed in foundation.cs.

We have a sub-optimal API in initWithURL:options:documentAttributes:error due to issues in iOS 9, which we aren't supporting anymore.

I started a patch to fix this:

https://gist.github.com/e5bf4dae26f2accdaf4b6a9de76ed206

but ran into time box issues and it started getting complicated in a mechanical PR. I decided to punt it for now, thus this issue.
@chamons chamons added the dotnet-pri2 .NET 6: want to have for stable release label Mar 24, 2022
@chamons chamons added this to the .NET 6 milestone Mar 24, 2022
@chamons
Copy link
Contributor Author

chamons commented Mar 24, 2022

FYI @rolfbjarne @dalexsoto

chamons added a commit to chamons/xamarin-macios that referenced this issue Mar 24, 2022
@chamons
Remove unnecessary conditional defines from foundation.cs
0d029b8 
- Unlike the rest of my recent work, this is all done by hand, so please review closely.
- Part of dotnet#14444
- Do note that there still are defines left, as there are many "almost same but not exactly" between platforms.
- Some of these only differ on ref/out
- Filed the rest as dotnet#14489
@chamons chamons mentioned this issue Mar 24, 2022
Merged
chamons added a commit that referenced this issue Mar 31, 2022
@chamons @mandel-macaque
Remove unnecessary conditional defines from foundation.cs (#14490)
7b0da17 
- Unlike the rest of my recent work, this is all done by hand, so please review closely.
- Part of #14444
- Do note that there still are defines left, as there are many "almost same but not exactly" between platforms.
- Some of these only differ on ref/out
- Filed the rest as #14489

Co-authored-by: Manuel de la Pena <[email protected]>
rolfbjarne added a commit to rolfbjarne/xamarin-macios that referenced this issue Apr 7, 2022
@rolfbjarne
[Foundation] Fix numerous binding issues in NSAttributedString. Fixes d…
32fd6f9 
…otnet#14489.

There were numerous differences in the managed API for NSAttributedString
between macOS and other platforms: most notably macOS would use 'out'
parameters, while the other platforms would use 'ref' parameters (possibly due
to this being very old API, and we didn't have proper support for 'out'
attributes when it was implemented).

This PR unifies the API between all platforms when it can (either now or in
XAMCORE_5_0).

It's hard to determine whether the changes don't break the API: api-diff is
happy locally, but the results on the bots will be the final determination.

Fixes dotnet#14489.
@rolfbjarne rolfbjarne mentioned this issue Apr 7, 2022
Closed
@rolfbjarne
Copy link
Member

I had a look at the NSAttributedString API, and there is significant room for improvement. That said, it seems to be possible to do it without breaking anything, so I'm postponing to .NET 7.

@rolfbjarne rolfbjarne modified the milestones: .NET 6, .NET 7 Apr 29, 2022
@rolfbjarne rolfbjarne added enhancement The issue or pull request is an enhancement and removed dotnet-pri2 .NET 6: want to have for stable release labels Apr 29, 2022
@rolfbjarne rolfbjarne changed the title [Potential NET6 API break] NSAttributedString ctor in foundation.cs Improve NSAttributedString ctors in foundation.cs Aug 26, 2022
@rolfbjarne rolfbjarne modified the milestones: .NET 7, Future Aug 26, 2022
rolfbjarne added a commit that referenced this issue Nov 15, 2022
@rolfbjarne
[Foundation] Enable nullability for NSAttributedString. (#16728)
ea17a27 
Part of #14489.

Co-authored-by: GitHub Actions Autoformatter <[email protected]>
@rolfbjarne rolfbjarne mentioned this issue Nov 23, 2022
Merged
rolfbjarne added a commit to rolfbjarne/xamarin-macios that referenced this issue Nov 23, 2022
@rolfbjarne
[Foundation] Remove some conditional logic from NSAttributedString's …
9ed8765 
…API definition.

This simplifies the code a little bit.

Ref: dotnet#14489
This was referenced Nov 23, 2022
Merged
Merged
rolfbjarne added a commit that referenced this issue Nov 28, 2022
@rolfbjarne
[Foundation] Merge multiple partial NSAttributedString definitions. (#…
7af808f 
…16889)

It just ends up being confusing and difficult to get a full picture of the type's
API definition if it's spread over multiple places.

Ref: #14489
rolfbjarne added a commit that referenced this issue Nov 28, 2022
@rolfbjarne
[Foundation] Remove some conditional logic from NSAttributedString's …
e9e5647 
…API definition. (#16882)

This simplifies the code a little bit.

Ref: #14489
rolfbjarne added a commit that referenced this issue Nov 29, 2022
@rolfbjarne
[Foundation] Share custom constructor code between iOS and macOS for …
5844f64 
…NSAttributedString. (#16880)

This also makes the set of constructors for NSAttributedString identical
between all platforms.

Ref: #14489
@rolfbjarne rolfbjarne mentioned this issue Dec 5, 2022
Merged
rolfbjarne added a commit that referenced this issue Dec 5, 2022
@rolfbjarne
[Foundation] Create a new NSAttributedStringDocument type to match Ap…
966933d 
…ple's headers. (#16952)

This also allows us to unify the code between all platforms.

There are no changes in the public API, because I'm only changing internal types.

Ref: #14489.
rolfbjarne added a commit to rolfbjarne/xamarin-macios that referenced this issue Dec 6, 2022
@rolfbjarne
[Foundation] Create a new NSAttributedStringDocumentAttributeKey enum…
9f61075 
… to match Apple's headers.

This allows us to unify the code between all platforms.

Also add all the NSAttributedStringDocumentAttributeKey values we haven't bound yet.

There are no changes in the public API, because I'm only changing internal types.

Ref: dotnet#14489.
@rolfbjarne rolfbjarne mentioned this issue Dec 6, 2022
Merged
rolfbjarne added a commit that referenced this issue Dec 7, 2022
@rolfbjarne
[Foundation] Create a new NSAttributedStringDocumentAttributeKey enum…
6581fcb 
… to match Apple's headers. (#16969)

This allows us to unify the code between all platforms.

Also add all the NSAttributedStringDocumentAttributeKey values we haven't bound yet.

There are no changes in the public API, because I'm only changing internal types.

Ref: #14489.
@rolfbjarne rolfbjarne mentioned this issue Dec 21, 2022
Merged
rolfbjarne added a commit that referenced this issue Dec 21, 2022
@rolfbjarne
[Foundation] Make NSAttributedStringDocumentType a strongly-typed enu…
68a08ad 
…m. (#17094)

This simplifies our code somewhat, and in XAMCORE_5_0 we can simplify
even more.

Ref: #14489
@tipa tipa mentioned this issue Nov 26, 2024
Merged
@rolfbjarne rolfbjarne mentioned this issue Nov 29, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement The issue or pull request is an enhancement
Projects
None yet
Milestone
Future
3 participants
@rolfbjarne @chamons @haritha-mohan