Allow Fragment::arg with nullable parameters.
#420
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
oldwomanjosiah
commented
Jun 15, 2022
RBusarow
approved these changes
Jul 27, 2022
There was a problem hiding this comment.
Thanks for the PR. I've added tests, added an overload for argOrNull to handle args which are missing entirely, and removed the Java overload.
The CI wasn't executing because by default, GitHub does not run actions against PRs from forks until they've been allowed by a repo admin. Aaand I didn't notice that I needed to approve it. 🤷 Sorry about that.
| * @param clazz type class expected for this argument | ||
| * @see FragmentInjectFactory for instantiating fragments with arguments | ||
| */ | ||
| public fun <A : Any?> Fragment.arg(bundleKey: String, clazz: Class<A>): Lazy<A> = lazy { |
There was a problem hiding this comment.
I'm not willing to introduce an overload for Java compatibility, considering that this library is tied to a Kotlin compiler plugin.
RBusarow
force-pushed
the
josiah/update_arg
branch
from
f16fcf1
to
ff03ca7
Compare
Remove the non-null restriction on `Fragment::arg` using an `isInstance` check instead of the `as?` cast. This requires more type information at runtime, so a `Class<A>` param is added as well as a convenience inline function with reified type parameter.
Update based on [this response](actions/checkout#551 (comment)). This may not work for private repos.
- remove overloaded java version - use KClass<*> type names for error messages (e.g., `kotlin.Int` vs `java.lang.Integer`) - fill out kdoc - add tests
RBusarow
force-pushed
the
josiah/update_arg
branch
from
ff03ca7
to
681bfe3
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Remove the non-null restriction on
Fragment::argusing anisInstancecheck instead of the
as?cast. This requires more type information atruntime, so a
Class<A>param is added as well as a convenience inlinefunction with reified type parameter.
I didn't see any tests for this function at the moment, so none were added. Just let me know if you would like this to change.
Binary incompatibility: Adds a new parameter to the only overload of
argvisible from Java.closes #414