-
Notifications
You must be signed in to change notification settings - Fork 12
Add AndroidApplicationStorytaleGradlePlugin #76
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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") { | ||
project.plugins.withId("org.jetbrains.compose") { | ||
val extension = | ||
project.extensions.create(STORYTALE_EXTENSION_NAME, StorytaleExtension::class.java, project) | ||
project.processConfigurations(extension) | ||
} | ||
val multiplatformEnabled = project.plugins.hasPlugin("org.jetbrains.kotlin.multiplatform") | ||
val androidApplicationEnabled = project.plugins.hasPlugin("com.android.application") | ||
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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Storytale strictly requires Did you want to make the new plugin to work on android without Compose Multiplatform? |
||
(multiplatformEnabled || androidApplicationEnabled) | ||
) { | ||
val extension = project.extensions.create( | ||
STORYTALE_EXTENSION_NAME, | ||
StorytaleExtension::class.java, | ||
project, | ||
) | ||
project.processConfigurations(extension) | ||
} | ||
} | ||
|
||
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @eymar Does There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
So yes,
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The same here. @congvc-dev could you use |
||
when (this) { | ||
extension.targets.forEach { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why was |
||
when (it) { | ||
is KotlinJsIrTarget -> | ||
when (wasmTargetType) { | ||
KotlinWasmTargetType.JS -> processWasmCompilation(extension, this) | ||
null -> processJsCompilation(extension, this) | ||
when (it.wasmTargetType) { | ||
KotlinWasmTargetType.JS -> processWasmCompilation(extension, it) | ||
null -> processJsCompilation(extension, it) | ||
else -> {} | ||
} | ||
is KotlinAndroidTarget -> processAndroidCompilation(extension, this) | ||
is KotlinJvmTarget -> processJvmCompilation(extension, this) | ||
is KotlinNativeTarget -> processNativeCompilation(extension, this) | ||
is KotlinAndroidTarget -> processAndroidCompilation(extension, it) | ||
is KotlinJvmTarget -> processJvmCompilation(extension, it) | ||
is KotlinNativeTarget -> processNativeCompilation(extension, it) | ||
} | ||
} | ||
} | ||
|
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.
@eymar, Am I right that
withId
works lazily, so that the order of the plugins application doesn't matter if we usewithId
?Uh oh!
There was an error while loading. Please reload this page.
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.
according to the documentation:
so yes, the order of
apply(plugin)
calls doesn't matter when usingwithId
.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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So, yep, this is what I'm afraid of. @congvc-dev, could you rework it using the
withId
approach?