[Foundation] Unify a few NSAttributedString constructor implementations. #16804

rolfbjarne · 2022-11-18T06:38:37Z

Unify the code for the following constructors:

NSAttributedString (NSData data, NSDictionary options, out NSDictionary resultDocumentAttributes, ref/out NSError error);
NSAttributedString (NSUrl url, NSAttributedStringDocumentAttributes options, out NSDictionary resultDocumentAttributes, ref/out NSError error);
NSAttributedString (NSData data, NSAttributedStringDocumentAttributes options, out NSDictionary resultDocumentAttributes, ref/out NSError error);

These functions use 'ref' arguments instead of 'out' arguments for mobile
platforms (likely due to the generator not having proper 'out' parameter
support when these functions were implemented), so improve to use 'out'
parameters in XAMCORE_5_0 (and macOS, where they already use 'out'
parameters).

Also fix nullability.

Ref: #15216

Unify the code for the following constructors: + NSAttributedString (NSData data, NSDictionary options, out NSDictionary resultDocumentAttributes, ref/out NSError error); + NSAttributedString (NSUrl url, NSAttributedStringDocumentAttributes options, out NSDictionary resultDocumentAttributes, ref/out NSError error); + NSAttributedString (NSData data, NSAttributedStringDocumentAttributes options, out NSDictionary resultDocumentAttributes, ref/out NSError error); These functions use 'ref' arguments instead of 'out' arguments for mobile platforms (likely due to the generator not having proper 'out' parameter support when these functions were implemented), so improve to use 'out' parameters in XAMCORE_5_0 (and macOS, where they already use 'out' parameters). Also fix nullability. Ref: xamarin#15216

rolfbjarne · 2022-11-18T16:13:28Z

Test failure is unrelated (https://github.com/xamarin/maccore/issues/2632).

…tor-unification-1

vs-mobiletools-engineering-service2 · 2022-11-21T11:20:15Z

✅ API diff for current PR / commit

Legacy Xamarin (No breaking changes)

~~iOS~~ (no change detected)
~~tvOS~~ (no change detected)
~~watchOS~~ (no change detected)
~~macOS~~ (no change detected)

NET (empty diffs)

~~iOS~~: (empty diff detected)
~~tvOS~~: (empty diff detected)
~~MacCatalyst~~: (empty diff detected)
~~macOS~~: (empty diff detected)

❗ API diff vs stable (Breaking changes)

Legacy Xamarin (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)

iOS: vsdrops gist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
tvOS: vsdrops gist (No breaking changes)
watchOS: vsdrops gist (No breaking changes)
macOS: vsdrops gist (No breaking changes)

.NET (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)

iOS: vsdrops gist (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
tvOS: vsdrops gist (No breaking changes)
MacCatalyst: vsdrops gist (No breaking changes)
macOS: vsdrops gist (No breaking changes)
Microsoft.iOS vs Microsoft.MacCatalyst: vsdrops gist

Legacy Xamarin (stable) vs .NET

iOS: vsdrops gist
tvOS: vsdrops gist
macOS: vsdrops gist

ℹ️ Generator diff

Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)

Pipeline on Agent
Hash: 2a49e315643fac5d1577fdc2559418484101ff1f [PR build]

vs-mobiletools-engineering-service2 · 2022-11-21T11:27:44Z

📚 [PR Build] Artifacts 📚

Artifacts were not provided.

Pipeline on Agent XAMBOT-1104.Monterey'
Hash: 2a49e315643fac5d1577fdc2559418484101ff1f [PR build]

vs-mobiletools-engineering-service2 · 2022-11-21T11:38:34Z

💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻

✅ All tests on macOS M1 - Mac Big Sur (11.5) passed.

Pipeline on Agent
Hash: 2a49e315643fac5d1577fdc2559418484101ff1f [PR build]

vs-mobiletools-engineering-service2 · 2022-11-21T17:50:01Z

🔥 [CI Build] Test results 🔥

Test results

❌ Tests failed on VSTS: simulator tests

0 tests crashed, 5 tests failed, 218 tests passed.

Failures

❌ bcl tests

5 tests failed, 64 tests passed.

[NUnit] Mono Mac OS X BCL tests group 3/Mac Full/Debug: Failed (Test run failed.
Tests run: 11328 Passed: 10552 Inconclusive: 0 Failed: 1 Ignored: 98)
[NUnit] Mono Mac OS X BCL tests group 3/Mac Modern/Debug: Failed (Test run failed.
Tests run: 8755 Passed: 8174 Inconclusive: 0 Failed: 1 Ignored: 46)
[NUnit] Mono BCL tests group 2/iOS Unified 64-bits - simulator/Debug: Failed
[NUnit] Mono BCL tests group 2/tvOS - simulator/Debug: Failed
[NUnit] Mono CorlibTests/watchOS 32-bits - simulator/Debug: Failed

Html Report (VSDrops) Download

Successes

✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests: All 1 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 7 tests passed. Html Report (VSDrops) Download
✅ framework: All 8 tests passed. Html Report (VSDrops) Download
✅ generator: All 2 tests passed. Html Report (VSDrops) Download
✅ interdependent_binding_projects: All 7 tests passed. Html Report (VSDrops) Download
✅ install_source: All 1 tests passed. Html Report (VSDrops) Download
✅ introspection: All 8 tests passed. Html Report (VSDrops) Download
✅ linker: All 65 tests passed. Html Report (VSDrops) Download
✅ mac_binding_project: All 1 tests passed. Html Report (VSDrops) Download
✅ mmp: All 2 tests passed. Html Report (VSDrops) Download
✅ mononative: All 12 tests passed. Html Report (VSDrops) Download
✅ monotouch: All 23 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ mtouch: All 1 tests passed. Html Report (VSDrops) Download
✅ xammac: All 3 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 8 tests passed. Html Report (VSDrops) Download
✅ xtro: All 2 tests passed. Html Report (VSDrops) Download

Pipeline on Agent
Hash: 2a49e315643fac5d1577fdc2559418484101ff1f [PR build]

rolfbjarne · 2022-11-21T20:04:27Z

Test failures are unrelated (https://github.com/xamarin/maccore/issues/2629).

…ns. (xamarin#16804) Unify the code for the following constructors: * NSAttributedString (NSData data, NSDictionary options, out NSDictionary resultDocumentAttributes, ref/out NSError error); * NSAttributedString (NSUrl url, NSAttributedStringDocumentAttributes options, out NSDictionary resultDocumentAttributes, ref/out NSError error); * NSAttributedString (NSData data, NSAttributedStringDocumentAttributes options, out NSDictionary resultDocumentAttributes, ref/out NSError error); These functions use 'ref' arguments instead of 'out' arguments for mobile platforms (likely due to the generator not having proper 'out' parameter support when these functions were implemented), so improve to use 'out' parameters in XAMCORE_5_0 (and macOS, where they already use 'out' parameters). Also fix nullability. Ref: xamarin#15216

tipa · 2024-11-26T08:52:31Z

@rolfbjarne can these constructors ever return a non-null NSError?
When I provide an invalid URL it throws an exception instead of returning null (what, from my understanding, is possible in the Apple-world, but not in .NET):

var s = new NSAttributedString(new NSUrl(""), new NSAttributedStringDocumentAttributes(), out NSError e);

or

var attributes = new NSAttributedStringDocumentAttributes { DocumentType = NSDocumentType.RTF, };
var s = new NSMutableAttributedString(new NSAttributedString("", attributes, out NSError _));

System.Exception: Could not initialize an instance of the type 'Foundation.NSAttributedString': the native 'initWithURL:options:documentAttributes:error:' method returned nil.

What is the correct way to use these constructors? Just wrap them in try-catch instead of inspecting the error variable?
Perhaps this issue is related?

Managed constructors can't fail gracefully, which means that constructors with an "out NSError" parameter doesn't make much sense. Instead bind these constructors using a factory method. Ref: #16804 (comment)

rolfbjarne · 2024-11-29T20:11:38Z

@tipa

What is the correct way to use these constructors? Just wrap them in try-catch instead of inspecting the error variable?

This is rather unfortunate, these constructors should really be bound as factory methods instead. For now, I guess the best is to wrap them in a try-catch, but then you won't be able to inspect the error variable.

I've created a PR to create factory methods for them instead: #21727

Perhaps this issue is related?

That's a fairly generic issue saying there are problems with the NSAttributedString ctors, which is true enough.

… (#21727) Managed constructors can't fail gracefully, which means that constructors with an "out NSError" parameter doesn't make much sense. Instead bind these constructors using a factory method. Ref: #16804 (comment) Fixes #14489.

Managed constructors can't fail gracefully, which means that constructors with an "out NSError" parameter doesn't make much sense. Instead bind these constructors using a factory method. References: * #16804 (comment) * #21727 Missing: * Manual tests.

…21803) Managed constructors can't fail gracefully, which means that constructors with an "out NSError" parameter doesn't make much sense. Instead bind these constructors using a factory method. References: * #16804 (comment) * #21727

This comment has been minimized.

Sign in to view

rolfbjarne changed the title ~~[Foundation] Unify a NSAttributedString few constructor implementations.~~ [Foundation] Unify a few NSAttributedString constructor implementations. Nov 18, 2022

tj-devel709 approved these changes Nov 19, 2022

View reviewed changes

rolfbjarne added 2 commits November 21, 2022 10:17

Fix breaking change.

fe8e03a

Merge remote-tracking branch 'origin/main' into nsattributed-string-c…

2a49e31

…tor-unification-1

rolfbjarne merged commit c1606d5 into xamarin:main Nov 21, 2022

rolfbjarne deleted the nsattributed-string-ctor-unification-1 branch November 21, 2022 20:05

rolfbjarne mentioned this pull request Nov 29, 2024

[Foundation] Improve the NSAttributedString constructors. Fixes #14489. #21727

Merged

rolfbjarne mentioned this pull request Dec 12, 2024

[Foundation] Fix/improve a few more NSAttributedString constructors. #21803

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Foundation] Unify a few NSAttributedString constructor implementations. #16804

[Foundation] Unify a few NSAttributedString constructor implementations. #16804

rolfbjarne commented Nov 18, 2022

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rolfbjarne commented Nov 18, 2022

vs-mobiletools-engineering-service2 commented Nov 21, 2022

vs-mobiletools-engineering-service2 commented Nov 21, 2022

vs-mobiletools-engineering-service2 commented Nov 21, 2022

vs-mobiletools-engineering-service2 commented Nov 21, 2022

rolfbjarne commented Nov 21, 2022

tipa commented Nov 26, 2024 •

edited

Loading

rolfbjarne commented Nov 29, 2024

[Foundation] Unify a few NSAttributedString constructor implementations. #16804

[Foundation] Unify a few NSAttributedString constructor implementations. #16804

Conversation

rolfbjarne commented Nov 18, 2022

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

rolfbjarne commented Nov 18, 2022

vs-mobiletools-engineering-service2 commented Nov 21, 2022

✅ API diff for current PR / commit

❗ API diff vs stable (Breaking changes)

ℹ️ Generator diff

vs-mobiletools-engineering-service2 commented Nov 21, 2022

📚 [PR Build] Artifacts 📚

vs-mobiletools-engineering-service2 commented Nov 21, 2022

💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻

vs-mobiletools-engineering-service2 commented Nov 21, 2022

🔥 [CI Build] Test results 🔥

Test results

Failures

❌ bcl tests

Successes

rolfbjarne commented Nov 21, 2022

tipa commented Nov 26, 2024 • edited Loading

rolfbjarne commented Nov 29, 2024

tipa commented Nov 26, 2024 •

edited

Loading