-
Notifications
You must be signed in to change notification settings - Fork 312
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
Add typedefs for completion blocks #339
Add typedefs for completion blocks #339
Conversation
Declare three block types in OnePasswordExtension.h, each with the name pattern "OnePassword<Kind>CompletionBlock". <Kind> is: * "Login" for blocks that pass a login item as an NSDictionary * "Success" for blocks that pass a `success` BOOL * "Extension" for blocks that pass an NSExtensionItem Adjust the immediately preceding header comment to warn framework repackagers about namespacing not only the class, but the new typedefs as well. Replace all explicit block types in method signatures (in both the header and implementation files) with equivalent typedef'd names. Since several methods exist in the implementation that aren't exposed in the header, and the assume-nonnull range does not extend throughout the implementation file, explicitly mark blocks in such methods `nonnull` (matching other arguments). Signed-off-by: Tim Ekl <lithium3141@gmail.com>
// and associated typedefs. You might to so by adding your own project prefix, e.g., | ||
// MyLibraryOnePasswordExtension. | ||
|
||
typedef void (^OnePasswordLoginCompletionBlock)(NSDictionary * __nullable loginDictionary, NSError * __nullable error); |
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.
Can we please rename OnePasswordLoginCompletionBlock
to something like OnePasswordLoginDictionaryCompletionBlock
?
Thanks so much for contributing @lithium3141! 👍 At first glance, the proposed changes look great, except for the couple of renaming issue (see the in-line comments). I just labeled the PR with I will keep you posted by commenting on this PR. In the meantime, if you have any other questions, please do not hesitate to ask. Cheers! |
Expand the Login and Extension completion block type names to contain full return type phrases: LoginDictionary and ExtensionItem. Signed-off-by: Tim Ekl <lithium3141@gmail.com>
Done in d5e4de7. Thanks for the quick reply! |
Thanks for updating your PR so quickly, @lithium3141 👍 After much consideration and testing with my colleague @nathanvf, we concluded that this is an Awesome addition to the App Extension API. I am therefore merging this PR right now! All the Best, |
Apple's documentation for declaring and creating blocks advocates for naming block types when they are used in multiple places. The current 1Password extension API "spells out" entire block signatures at each use, despite having only three distinct completion block types.
Including
typedef
s for these three block types would significantly reduce line noise across seven method declarations in OnePasswordExtension.h, as well as several more in the corresponding implementation file.This branch declares these three types in OnePasswordExtension.h, each with the name pattern
OnePassword<Kind>CompletionBlock
. "Kind" is:Login
for blocks that pass a login item as an NSDictionarySuccess
for blocks that pass asuccess
BOOLExtension
for blocks that pass an NSExtensionItemAll "spelled out" block types in method signatures (in both the header and implementation files) are replaced with equivalent
typedef
'd names. Since several methods exist in the implementation that aren't exposed in the header, and the assume-nonnull range does not extend throughout the implementation file, explicitly mark blocks in such methodsnonnull
(matching other arguments).This change does slightly increase the surface area of symbols needing prefixing for developers intending to redistribute this source as part of a framework. I think this is an acceptable tradeoff, but am happy to hear out concerns in that direction. (The branch also adjusts the comment warning about prefixing the class to also mention the new
typedef
s.)