-
Notifications
You must be signed in to change notification settings - Fork 511
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
[PDFKit] Updating Xcode13 Beta 1 #11987
Conversation
❌ [PR Build] Tests failed on Build ❌Tests failed on Build. API diff✅ API Diff from stable View API diffAPI & Generator diffℹ️ API Diff (from PR only) (please review changes) GitHub pagesResults can be found in the following github pages (it might take some time to publish): Test results1 tests failed, 85 tests passed.Failed tests
Pipeline on Agent XAMBOT-1104.BigSur' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wasn't sure if the PdfAccessPermissions type should be ulong.
Sharpie gave it the ulong type and used the (1uL << #) but the original header used NSUInteger which according to this should be nuint
ulong
is correct. We'd like to use nuint
, but we can't do that for enums, C# doesn't allow it (only [s]byte, [u]short, [u]int and [u]long are allowed as base types for enums), so we go with the second best option: the 64-bit type. It's the correct type for 64-bit platforms (which will one day be all platforms), and then we add the [Native]
attribute to tell the generator and our runtime that it should be treated as a 32-bit value on 32-bit platforms.
src/pdfkit.cs
Outdated
|
||
[iOS (15,0), Mac (12,0), MacCatalyst (15,0)] | ||
[Field ("PDFDocumentAccessPermissionsOption")] | ||
NSString AccessPermissionsOption { get; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that last field should with inside PdfDocumentWriteOptionKeys
the hint is that the previous fields (in the headers) are similarly named and (in binding files) already grouped into their own type
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and also update the PdfDocumentWriteOptions
strong dictionary :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@spouliot I added PDFDocumentAccessPermissionsOption
to PdfDocumentWriteOptionKeys
and PdfDocumentWriteOptions
. I followed along with the pattern of the other elements in there, but not sure why the "+PDFKit" is there or if I needed to add that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops forgot to run tests, I'll do that now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay the tests look good!
@@ -434,6 +438,7 @@ interface PdfDocumentWriteOptions { | |||
|
|||
string OwnerPassword { get; set; } | |||
string UserPassword { get; set; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is missing the availability attrs which should be the same as those of the key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
without them the developer won't know they are not available (since the keys are not public)
and they (internal) keys also needs them otherwise introspection would not be able to do its job :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated :)
✅ [PR Build] Tests passed on Build. ✅Tests passed on Build. API diff✅ API Diff from stable View API diffAPI & Generator diffℹ️ API Diff (from PR only) (please review changes) GitHub pagesResults can be found in the following github pages (it might take some time to publish): 🎉 All 86 tests passed 🎉Pipeline on Agent XAMBOT-1096.BigSur' |
Wasn't sure if the PdfAccessPermissions type should be ulong.
Sharpie gave it the ulong type and used the
(1uL << #)
but the original header used NSUInteger which according to this should benuint