Add AndroidApplicationStorytaleGradlePlugin #76
Conversation
...e-plugin/src/main/kotlin/org/jetbrains/compose/storytale/plugin/AndroidStorytaleExtension.kt
Outdated
Show resolved
Hide resolved
|
By the way, @congvc-dev, I haven't found your contacts, so I will put them here. |
|
I think im at my limit of knowledge here. Somehow my android app module doesn't register |
|
Hi @congvc-dev 👋, Thank you for your contribution to this issue! I also tried to do some research for android projects, it seems that you can't directly add a custom 
but i'm currently not sure how to configure the existing Gradle plugin task for the corresponding flavor. Also, I think we might need to do some handling in For example, the task |
|
@whitescent, good approach! Thanks for untying the knot for me. I'll try look into it later this weekend |
|
@JSMonk, can you shoot me a DM? here is my profile in kotlinlang slack |
| @@ -14,12 +14,20 @@ import org.jetbrains.kotlin.gradle.targets.jvm.KotlinJvmTarget | |||
| class StorytaleGradlePlugin : KotlinCompilerPluginSupportPlugin { | |||
|
|
|||
| override fun apply(project: Project) { | |||
| project.plugins.withId("org.jetbrains.kotlin.multiplatform") { | |||
There was a problem hiding this comment.
@eymar, Am I right that withId works lazily, so that the order of the plugins application doesn't matter if we use withId?
There was a problem hiding this comment.
according to the documentation:
* Executes or registers an action for a plugin with given id.
* If the plugin was already applied, the action is executed.
* If the plugin is applied sometime later the action will be executed after the plugin is applied.
* If the plugin is never applied, the action is never executed.
so yes, the order of apply(plugin) calls doesn't matter when using withId.
There was a problem hiding this comment.
the new implementation I see on the right side would require a strict order of plugins: Storytale would need to be registered after all other plugins - it's error prone. Ideally the order shouldn't matter.
Using nested withId is a solution which come to my mind. And btw I see it used to be here below.
There was a problem hiding this comment.
So, yep, this is what I'm afraid of. @congvc-dev, could you rework it using the withId approach?
| @@ -32,17 +40,17 @@ class StorytaleGradlePlugin : KotlinCompilerPluginSupportPlugin { | |||
| override fun applyToCompilation(kotlinCompilation: KotlinCompilation<*>) = kotlinCompilation.target.project.provider { emptyList<SubpluginOption>() } | |||
|
|
|||
| private fun Project.processConfigurations(extension: StorytaleExtension) { | |||
| extension.targets.all { | |||
There was a problem hiding this comment.
@eymar Does all also work lazily?
There was a problem hiding this comment.
Executes the given action against all objects in this collection, and any objects subsequently added to this collection.
So yes, all {} will be invoked on all targets added before and after.
forEach will just iterate on the current state of the collection.
There was a problem hiding this comment.
The same here. @congvc-dev could you use all instead of the forEach?
| val composeEnabled = project.plugins.hasPlugin("org.jetbrains.compose") | ||
| val composePluginEnabled = project.plugins.hasPlugin("org.jetbrains.kotlin.plugin.compose") | ||
| if ( | ||
| (composeEnabled || composePluginEnabled) && |
There was a problem hiding this comment.
Storytale strictly requires org.jetbrains.compose rather than org.jetbrains.kotlin.plugin.compose. So || is rather incorrect.
Did you want to make the new plugin to work on android without Compose Multiplatform?
| @@ -32,17 +40,17 @@ class StorytaleGradlePlugin : KotlinCompilerPluginSupportPlugin { | |||
| override fun applyToCompilation(kotlinCompilation: KotlinCompilation<*>) = kotlinCompilation.target.project.provider { emptyList<SubpluginOption>() } | |||
|
|
|||
| private fun Project.processConfigurations(extension: StorytaleExtension) { | |||
| extension.targets.all { | |||
| when (this) { | |||
| extension.targets.forEach { | |||
There was a problem hiding this comment.
why was all replaced by forEach?
this PR fixes long awaited #38.
I tested with my local android app module but didn't commit because it isn't necessarily.