ADFA-3468: Add plugin template contribution API#1121
Closed
Daniel-ADFA wants to merge 1 commit into
Closed
Conversation
Contributor
📝 WalkthroughWalkthroughThis change introduces a plugin-based template management subsystem, enabling plugins to create, register, and manage IDE template files (.cgt format). It includes new builder APIs for template construction, plugin lifecycle integration for template extraction and cleanup, and conditional UI controls for template parameters. Changes
Sequence DiagramsequenceDiagram
participant Plugin as Plugin Load
participant PluginMgr as PluginManager
participant ProjectMgr as PluginProjectManager
participant FS as File System
participant TemplateSvc as IdeTemplateServiceImpl
participant UI as UI/Listener
Plugin->>PluginMgr: loadPlugin(pluginId)
PluginMgr->>ProjectMgr: extractBundledCgtTemplates(pluginId)
ProjectMgr->>FS: scan classLoader for .cgt files
FS-->>ProjectMgr: list .cgt resources
ProjectMgr->>FS: copy plugin_{pluginId}_*.cgt to TEMPLATES_DIR
PluginMgr->>TemplateSvc: createService(pluginId)
TemplateSvc->>TemplateSvc: initialize with plugin prefix
PluginMgr->>UI: invoke templateReloadListener()
Plugin->>TemplateSvc: registerTemplate(cgtFile)
TemplateSvc->>TemplateSvc: validatePermissions()
TemplateSvc->>FS: copy .cgt with plugin prefix
TemplateSvc->>UI: onTemplatesChanged()
Plugin->>PluginMgr: unloadPlugin(pluginId)
PluginMgr->>TemplateSvc: cleanupAllTemplates()
TemplateSvc->>FS: delete plugin_{pluginId}_*.cgt files
TemplateSvc->>UI: onTemplatesChanged()
PluginMgr->>ProjectMgr: cleanupPluginTemplates(pluginId)
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt (1)
402-404:⚠️ Potential issue | 🟠 MajorAdd missing call to
extractBundledCgtTemplatesduring plugin load.The
PluginProjectManager.extractBundledCgtTemplates(pluginId, classLoader)is never invoked during plugin loading, creating an asymmetric lifecycle:cleanupPluginTemplatesis called during unload (line 471) but the corresponding extraction call is absent during load.Both methods share the same naming convention and reload listener, indicating they're designed as paired operations. Without the extraction call, plugins cannot bundle
.cgttemplate files that should be auto-extracted.Add
PluginProjectManager.getInstance().extractBundledCgtTemplates(manifest.id, classLoader)in the loadPlugin method after successful plugin initialization.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt` around lines 402 - 404, The plugin load path in loadPlugin creates and registers LoadedPlugin but never calls PluginProjectManager.getInstance().extractBundledCgtTemplates(manifest.id, classLoader), causing bundled .cgt templates to never be extracted (cleanupPluginTemplates is called on unload). After successful plugin initialization and after creating/putting the LoadedPlugin (the block that constructs LoadedPlugin and sets loadedPlugins[manifest.id]), add a call to PluginProjectManager.getInstance().extractBundledCgtTemplates(manifest.id, classLoader) so extraction runs during load and matches cleanupPluginTemplates during unload.
🧹 Nitpick comments (6)
plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/services/IdeTemplateService.kt (1)
6-19: Clean interface design.The interface is well-structured with a clear contract for template management. Consider adding KDoc comments for each method to document expected behavior, parameters, and return values for plugin developers.
📝 Example documentation
interface IdeTemplateService { + /** + * Creates a new template builder for constructing a .cgt template archive. + * `@param` name The display name of the template + * `@return` A builder instance for fluent template construction + */ fun createTemplateBuilder(name: String): CgtTemplateBuilder + /** + * Registers a .cgt template file with the IDE. + * `@param` cgtFile The template file to register (must exist and have .cgt extension) + * `@return` true if registration succeeded, false otherwise + */ fun registerTemplate(cgtFile: File): Boolean🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/services/IdeTemplateService.kt` around lines 6 - 19, Add KDoc comments to the IdeTemplateService interface and each method (createTemplateBuilder, registerTemplate, unregisterTemplate, isTemplateRegistered, getRegisteredTemplates, reloadTemplates) to document expected behavior, parameters and return values for plugin authors; for each method include a brief description, `@param` entries for parameters (e.g., name, cgtFile, templateFileName) and `@return` descriptions for Boolean/List results, and note any side-effects or exceptions (e.g., reloadTemplates mutates internal state) so implementers and callers have clear contract guidance.plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/templates/CgtTemplateBuilder.kt (2)
223-226:BinaryFileEntrydoesn't overrideequals/hashCodeforByteArray.The
data class BinaryFileEntryincludes aByteArrayproperty. Kotlin data classes use structural equality, butByteArray.equals()uses referential equality. This can cause unexpected behavior if these entries are compared or used in collections.📝 Note
If
BinaryFileEntryinstances are never compared or stored in sets/maps, this is fine. Otherwise, consider overridingequals/hashCodeto usecontentEqualsfor the byte array, or convert to a regular class.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/templates/CgtTemplateBuilder.kt` around lines 223 - 226, BinaryFileEntry uses a ByteArray which defaults to referential equality causing incorrect comparisons/hashing; update BinaryFileEntry to override equals and hashCode (or convert to a regular class) so the bytes are compared by content: implement equals to use bytes.contentEquals(other.bytes) and compare path, and implement hashCode to combine path.hashCode() with bytes.contentHashCode(); ensure the class still has same property names (BinaryFileEntry, path, bytes) so usages remain unchanged.
158-188: The"placeholder": nullfield is not in the expected JSON schema.Line 172 adds
"placeholder": nullwhich doesn't exist inParametersJson(seetemplates-impl/.../ZipJson.kt). While GSON ignores unknown fields by default, this is a fragile workaround for trailing comma handling.Consider using a JSON library (like Gson or kotlinx.serialization) to generate the metadata JSON safely, or restructure the string building to properly handle trailing commas.
♻️ Alternative approach using buildList
private fun writeMetadata(zip: ZipOutputStream, dirName: String) { val requiredParts = buildList { add(""""appName": {"identifier": "APP_NAME"}""") if (showPackageName) add(""""packageName": {"identifier": "PACKAGE_NAME"}""") add(""""saveLocation": {"identifier": "SAVE_LOCATION"}""") } val requiredBlock = requiredParts.joinToString(",\n ") // ... similar pattern for other blocks }plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt (1)
471-476: Redundant template cleanup calls.Both
PluginProjectManager.cleanupPluginTemplates(pluginId)andIdeTemplateServiceImpl.cleanupAllTemplates()delete files matching the same prefix pattern (plugin_{pluginId}_) fromEnvironment.TEMPLATES_DIR. The second call becomes a no-op after the first completes.Consider consolidating to a single cleanup mechanism or documenting the intent if these are meant to handle different scenarios.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt` around lines 471 - 476, The two successive cleanup calls are redundant: PluginProjectManager.getInstance().cleanupPluginTemplates(pluginId) and IdeTemplateServiceImpl.cleanupAllTemplates() both remove files with the same prefix (plugin_{pluginId}_) from Environment.TEMPLATES_DIR; choose one canonical cleanup path (either keep the specialized PluginProjectManager.cleanupPluginTemplates(pluginId) and remove the IdeTemplateServiceImpl.cleanupAllTemplates() call, or make cleanupAllTemplates() accept a pluginId/targeted scope and call that instead) or add a clear comment documenting why both are required for different scenarios; update the code to call only the consolidated method (or adjust signature/behavior of IdeTemplateServiceImpl.cleanupAllTemplates) and remove the no-op duplicate to avoid needless work.plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeTemplateServiceImpl.kt (2)
53-66: Consider logging whenunregisterTemplateis called for non-existent file.Line 55 silently returns
falsewhen the file doesn't exist. A debug log would help troubleshoot scenarios where a plugin expects a template to exist but it doesn't.📝 Proposed enhancement
override fun unregisterTemplate(templateFileName: String): Boolean { val destFile = File(Environment.TEMPLATES_DIR, prefixedName(templateFileName)) - if (!destFile.exists()) return false + if (!destFile.exists()) { + log.debug("Template not found for unregistration: ${destFile.name}") + return false + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeTemplateServiceImpl.kt` around lines 53 - 66, When unregisterTemplate is called and the target destFile (constructed via File(Environment.TEMPLATES_DIR, prefixedName(templateFileName))) does not exist, add a debug/info log before returning false to record that the template was missing for this pluginId; keep the early return behavior but ensure you log a clear message like "Plugin {pluginId} attempted to unregister missing template: {destFile.name}" so callers can trace why false was returned without changing onTemplatesChanged() or the existing try/catch.
30-51: Consider narrowing exception handling inregisterTemplate.The broad
catch (e: Exception)on line 47 catches all exception types. Based on learnings, prefer catching specific exceptions likeIOExceptionorSecurityExceptionthat are expected from file operations. This aligns with fail-fast behavior during development.♻️ Proposed fix
return try { cgtFile.copyTo(destFile, overwrite = true) log.info("Plugin $pluginId registered template: ${destFile.name}") onTemplatesChanged() true - } catch (e: Exception) { + } catch (e: IOException) { log.error("Failed to register template for plugin $pluginId", e) false }Based on learnings: "prefer narrow exception handling that catches only the specific exception type reported in crashes (such as IllegalArgumentException) instead of a broad catch-all".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeTemplateServiceImpl.kt` around lines 30 - 51, The registerTemplate method currently catches all exceptions with catch (e: Exception); narrow this to only the expected file-operation exceptions (e.g., catch IOException and catch SecurityException, and optionally catch IllegalArgumentException if copyTo can throw it) so we handle known failure modes and log them, then return false; rethrow or let other unexpected exceptions propagate instead of swallowing them. Update the try/catch around cgtFile.copyTo(...) in registerTemplate to use specific catches (IOException, SecurityException[, IllegalArgumentException]) that call log.error with the exception and return false, leaving any other exceptions unhandled.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/templates/CgtTemplateBuilder.kt`:
- Around line 80-95: The current build() creates dirName by stripping ASCII
alphanumerics from templateName (templateName.replace(Regex("[^a-zA-Z0-9]"),
"")), which can produce an empty dirName for non-ASCII names; update build() to
ensure dirName is never empty by falling back to a deterministic safe name (for
example: use a normalized/slugified Unicode-safe version of templateName or, if
that results in empty, a short hash/UUID-based fallback like "template_<hash>"),
and then use that dirName for Zip entries; change only the dirName generation
logic in build() so
writeIndex/writeMetadata/writeThumbnail/writeTemplateFiles/writeStaticFiles/writeBinaryFiles
receive a non-empty, filesystem-safe dirName.
- Around line 39-41: thumbnailFromAssets currently calls
context.androidContext.assets.open(assetPath) which can throw IOException; wrap
the asset open/read in a try/catch that catches IOException and rethrows a clear
unchecked exception (e.g., IllegalArgumentException or RuntimeException)
containing the assetPath and the original exception so callers get actionable
info, or alternatively log the error via PluginContext before rethrowing; update
the body of thumbnailFromAssets to perform the try/catch around the open/read
and set this.thumbnail only on success, referencing thumbnailFromAssets,
PluginContext, and assetPath in the error message.
- Around line 97-105: toPebbleSyntax currently double-escapes already-escaped
Pebble sequences (e.g., "${{") because sequential replace calls collide; fix by
implementing a three-step placeholder approach inside toPebbleSyntax: first
replace already-escaped sequences ("${{", "${%", "${#}") with unique temporary
tokens, then perform the normal escaping for raw Pebble syntax ("{{", "{%",
"{#"), and finally restore the temporary tokens back to their original escaped
forms; update the function to use distinct placeholder names to avoid collisions
and ensure all three patterns ({{, {%, {#}) and their pre-escaped forms are
handled.
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/project/PluginProjectManager.kt`:
- Around line 43-69: The extractCgtFromPath function relies on
classLoader.getResourceAsStream(basePath) to enumerate directory entries which
is unreliable; change it to read an explicit manifest/index file (e.g.,
"$basePath/index.txt" or "templates/index.txt") via
classLoader.getResourceAsStream(manifestPath) and parse its newline-separated
entries, then for each entry call
classLoader.getResourceAsStream("$basePath$entry") as you already do; update the
logic around entries (and the emptyList() fallback) to fail gracefully if the
manifest is missing and document or log that the plugin must include the
manifest; keep references to CGT_EXTENSION, PLUGIN_CGT_PREFIX and the destFile
creation intact.
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeTemplateServiceImpl.kt`:
- Around line 84-95: In cleanupAllTemplates(), replace the silent files.forEach
{ it.delete() } with a loop that captures the boolean result of File.delete();
for each delete() that returns false, emit a warning log mentioning the file
name (use the class logger/ log or existing logging mechanism), and track
whether any file was actually deleted so onTemplatesChanged() is called only if
at least one deletion succeeded; reference the methods/values prefixedName(""),
Environment.TEMPLATES_DIR, CGT_EXTENSION and the onTemplatesChanged() call when
making the change.
In `@templates-api/src/main/java/com/itsaky/androidide/templates/base/base.kt`:
- Around line 346-351: The project name→packageName synchronization is disabled
when showPackageName is false, leaving packageName.value stale for later
consumers (e.g., ModuleTemplateData, ZipRecipeExecutor); move or restore the
observer so projectName.observe always updates packageName by calling
AndroidUtils.appNameToPackageName and packageName.setValue regardless of
showPackageName, i.e., remove the conditional around projectName.observe (or
attach a separate unconditional listener) so packageName.value stays in sync
when projectName changes even if the package input is hidden.
---
Outside diff comments:
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt`:
- Around line 402-404: The plugin load path in loadPlugin creates and registers
LoadedPlugin but never calls
PluginProjectManager.getInstance().extractBundledCgtTemplates(manifest.id,
classLoader), causing bundled .cgt templates to never be extracted
(cleanupPluginTemplates is called on unload). After successful plugin
initialization and after creating/putting the LoadedPlugin (the block that
constructs LoadedPlugin and sets loadedPlugins[manifest.id]), add a call to
PluginProjectManager.getInstance().extractBundledCgtTemplates(manifest.id,
classLoader) so extraction runs during load and matches cleanupPluginTemplates
during unload.
---
Nitpick comments:
In
`@plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/services/IdeTemplateService.kt`:
- Around line 6-19: Add KDoc comments to the IdeTemplateService interface and
each method (createTemplateBuilder, registerTemplate, unregisterTemplate,
isTemplateRegistered, getRegisteredTemplates, reloadTemplates) to document
expected behavior, parameters and return values for plugin authors; for each
method include a brief description, `@param` entries for parameters (e.g., name,
cgtFile, templateFileName) and `@return` descriptions for Boolean/List results,
and note any side-effects or exceptions (e.g., reloadTemplates mutates internal
state) so implementers and callers have clear contract guidance.
In
`@plugin-api/src/main/kotlin/com/itsaky/androidide/plugins/templates/CgtTemplateBuilder.kt`:
- Around line 223-226: BinaryFileEntry uses a ByteArray which defaults to
referential equality causing incorrect comparisons/hashing; update
BinaryFileEntry to override equals and hashCode (or convert to a regular class)
so the bytes are compared by content: implement equals to use
bytes.contentEquals(other.bytes) and compare path, and implement hashCode to
combine path.hashCode() with bytes.contentHashCode(); ensure the class still has
same property names (BinaryFileEntry, path, bytes) so usages remain unchanged.
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.kt`:
- Around line 471-476: The two successive cleanup calls are redundant:
PluginProjectManager.getInstance().cleanupPluginTemplates(pluginId) and
IdeTemplateServiceImpl.cleanupAllTemplates() both remove files with the same
prefix (plugin_{pluginId}_) from Environment.TEMPLATES_DIR; choose one canonical
cleanup path (either keep the specialized
PluginProjectManager.cleanupPluginTemplates(pluginId) and remove the
IdeTemplateServiceImpl.cleanupAllTemplates() call, or make cleanupAllTemplates()
accept a pluginId/targeted scope and call that instead) or add a clear comment
documenting why both are required for different scenarios; update the code to
call only the consolidated method (or adjust signature/behavior of
IdeTemplateServiceImpl.cleanupAllTemplates) and remove the no-op duplicate to
avoid needless work.
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeTemplateServiceImpl.kt`:
- Around line 53-66: When unregisterTemplate is called and the target destFile
(constructed via File(Environment.TEMPLATES_DIR,
prefixedName(templateFileName))) does not exist, add a debug/info log before
returning false to record that the template was missing for this pluginId; keep
the early return behavior but ensure you log a clear message like "Plugin
{pluginId} attempted to unregister missing template: {destFile.name}" so callers
can trace why false was returned without changing onTemplatesChanged() or the
existing try/catch.
- Around line 30-51: The registerTemplate method currently catches all
exceptions with catch (e: Exception); narrow this to only the expected
file-operation exceptions (e.g., catch IOException and catch SecurityException,
and optionally catch IllegalArgumentException if copyTo can throw it) so we
handle known failure modes and log them, then return false; rethrow or let other
unexpected exceptions propagate instead of swallowing them. Update the try/catch
around cgtFile.copyTo(...) in registerTemplate to use specific catches
(IOException, SecurityException[, IllegalArgumentException]) that call log.error
with the exception and return false, leaving any other exceptions unhandled.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6d156879-f93e-411f-b2d4-8497c9c9f3db
📒 Files selected for processing (11)
app/src/main/java/com/itsaky/androidide/fragments/TemplateListFragment.ktplugin-api/src/main/kotlin/com/itsaky/androidide/plugins/services/IdeTemplateService.ktplugin-api/src/main/kotlin/com/itsaky/androidide/plugins/templates/CgtTemplateBuilder.ktplugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/core/PluginManager.ktplugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/project/PluginProjectManager.ktplugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeTemplateServiceImpl.kttemplates-api/src/main/java/com/itsaky/androidide/templates/base/ProjectTemplateBuilder.kttemplates-api/src/main/java/com/itsaky/androidide/templates/base/base.kttemplates-impl/src/main/java/com/itsaky/androidide/templates/impl/TemplateProviderImpl.kttemplates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipJson.kttemplates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipTemplateReader.kt
💤 Files with no reviewable changes (1)
- app/src/main/java/com/itsaky/androidide/fragments/TemplateListFragment.kt
Comment on lines
+84
to
+95
| fun cleanupAllTemplates() { | ||
| val prefix = prefixedName("") | ||
| val files = Environment.TEMPLATES_DIR | ||
| .listFiles { file -> file.name.startsWith(prefix) && file.extension == CGT_EXTENSION } | ||
| ?: return | ||
|
|
||
| files.forEach { it.delete() } | ||
|
|
||
| if (files.isNotEmpty()) { | ||
| onTemplatesChanged() | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Delete failures are silently ignored in cleanupAllTemplates.
The files.forEach { it.delete() } on line 90 doesn't check or log whether deletion succeeded. A file could remain due to permissions or locks, leaving stale templates.
🛡️ Proposed fix to log delete failures
fun cleanupAllTemplates() {
val prefix = prefixedName("")
val files = Environment.TEMPLATES_DIR
.listFiles { file -> file.name.startsWith(prefix) && file.extension == CGT_EXTENSION }
?: return
- files.forEach { it.delete() }
+ var deletedCount = 0
+ files.forEach { file ->
+ if (file.delete()) {
+ deletedCount++
+ } else {
+ log.warn("Failed to delete template file: ${file.name}")
+ }
+ }
- if (files.isNotEmpty()) {
+ if (deletedCount > 0) {
+ log.info("Cleaned up $deletedCount template files for plugin $pluginId")
onTemplatesChanged()
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fun cleanupAllTemplates() { | |
| val prefix = prefixedName("") | |
| val files = Environment.TEMPLATES_DIR | |
| .listFiles { file -> file.name.startsWith(prefix) && file.extension == CGT_EXTENSION } | |
| ?: return | |
| files.forEach { it.delete() } | |
| if (files.isNotEmpty()) { | |
| onTemplatesChanged() | |
| } | |
| } | |
| fun cleanupAllTemplates() { | |
| val prefix = prefixedName("") | |
| val files = Environment.TEMPLATES_DIR | |
| .listFiles { file -> file.name.startsWith(prefix) && file.extension == CGT_EXTENSION } | |
| ?: return | |
| var deletedCount = 0 | |
| files.forEach { file -> | |
| if (file.delete()) { | |
| deletedCount++ | |
| } else { | |
| log.warn("Failed to delete template file: ${file.name}") | |
| } | |
| } | |
| if (deletedCount > 0) { | |
| log.info("Cleaned up $deletedCount template files for plugin $pluginId") | |
| onTemplatesChanged() | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@plugin-manager/src/main/kotlin/com/itsaky/androidide/plugins/manager/services/IdeTemplateServiceImpl.kt`
around lines 84 - 95, In cleanupAllTemplates(), replace the silent files.forEach
{ it.delete() } with a loop that captures the boolean result of File.delete();
for each delete() that returns false, emit a warning log mentioning the file
name (use the class logger/ log or existing logging mechanism), and track
whether any file was actually deleted so onTemplatesChanged() is called only if
at least one deletion succeeded; reference the methods/values prefixedName(""),
Environment.TEMPLATES_DIR, CGT_EXTENSION and the onTemplatesChanged() call when
making the change.
Comment on lines
+346
to
351
| if (showPackageName) { | ||
| projectName.observe { name -> | ||
| val newPackage = AndroidUtils.appNameToPackageName(name.value, packageName.value) | ||
| packageName.setValue(newPackage) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Keep package value synced even when the package field is hidden.
On Line 346, disabling the observer when showPackageName=false leaves packageName.value stale/default, but that value is still consumed later (e.g., ModuleTemplateData on Line 412 and ZIP path replacement in ZipRecipeExecutor). This can generate incorrect package paths/content for hidden-package templates.
Suggested fix
- if (showPackageName) {
- projectName.observe { name ->
- val newPackage = AndroidUtils.appNameToPackageName(name.value, packageName.value)
- packageName.setValue(newPackage)
- }
- }
+ projectName.observe { name ->
+ val newPackage = AndroidUtils.appNameToPackageName(name.value, packageName.value)
+ packageName.setValue(newPackage)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (showPackageName) { | |
| projectName.observe { name -> | |
| val newPackage = AndroidUtils.appNameToPackageName(name.value, packageName.value) | |
| packageName.setValue(newPackage) | |
| } | |
| } | |
| projectName.observe { name -> | |
| val newPackage = AndroidUtils.appNameToPackageName(name.value, packageName.value) | |
| packageName.setValue(newPackage) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@templates-api/src/main/java/com/itsaky/androidide/templates/base/base.kt`
around lines 346 - 351, The project name→packageName synchronization is disabled
when showPackageName is false, leaving packageName.value stale for later
consumers (e.g., ModuleTemplateData, ZipRecipeExecutor); move or restore the
observer so projectName.observe always updates packageName by calling
AndroidUtils.appNameToPackageName and packageName.setValue regardless of
showPackageName, i.e., remove the conditional around projectName.observe (or
attach a separate unconditional listener) so packageName.value stays in sync
when projectName changes even if the package input is hidden.
jomen-adfa
approved these changes
Mar 26, 2026
Daniel-ADFA
changed the title
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Screen.Recording.2026-03-26.at.01.15.28.mov