Open
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughRelease NotesFeatures
API Changes
Security & Infrastructure
Internal Changes
|
| Cohort / File(s) | Summary |
|---|---|
ZIP Template Core templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipJson.kt, .../ZipTemplateConstants.kt, .../ZipTemplateReader.kt, .../ZipRecipeExecutor.kt |
Add JSON metadata models, constants, ZIP reader, and Pebble-based recipe executor to load and render templates from zip archives, handle thumbnails, parameters, and language-aware file skipping. |
Core Template API templates-api/src/main/java/com/itsaky/androidide/templates/template.kt, templates-api/src/main/java/com/itsaky/androidide/templates/base/base.kt |
Extend Template/ProjectTemplate/Builder with templateNameStr and thumbData; make language/minSdk nullable; add baseZipProject() helper for zip-backed templates. |
Template Provider templates-impl/src/main/java/com/itsaky/androidide/templates/impl/TemplateProviderImpl.kt |
Replace hardcoded templates with archive-driven loading from Environment.TEMPLATES_DIR, add reload flow, per-archive logging, error isolation, and getTemplate(id) accessor. |
Builder & Module Changes templates-api/src/main/java/com/itsaky/androidide/templates/base/ProjectTemplateBuilder.kt, .../modules/android/buildGradle.kt |
Pass templateNameStr and thumbData when building templates; make Gradle generation null-safe for minSdk (minSdk?.api). |
UI & Adapters app/src/main/java/com/itsaky/androidide/adapters/TemplateListAdapter.kt, app/src/main/java/com/itsaky/androidide/fragments/TemplateDetailsFragment.kt |
Switch display to templateNameStr, load thumbnails via Glide when available, add debug logging, and show language with null-safe fallback. |
Assets & Environment common/src/main/java/com/itsaky/androidide/utils/Environment.java, app/src/main/java/com/itsaky/androidide/assets/*, composite-builds/.../constants.kt, gradle/libs.versions.toml, templates-impl/build.gradle.kts, templates-api/build.gradle.kts |
Add Environment.TEMPLATES_DIR; introduce template archive constants and assets (core.cgt, core.cgt.br); update installers to extract/install core archive; add Pebble and Gson deps and constants. |
Sequence Diagram
sequenceDiagram
participant Client
participant TemplateProviderImpl
participant ZipTemplateReader
participant ZipFile
participant ZipRecipeExecutor
participant PebbleEngine
participant FileSystem
Client->>TemplateProviderImpl: getTemplates()
TemplateProviderImpl->>FileSystem: list files in Environment.TEMPLATES_DIR
loop per archive
TemplateProviderImpl->>ZipTemplateReader: read(zipFile)
ZipTemplateReader->>ZipFile: open archive & read templates.json
loop per template ref
ZipTemplateReader->>ZipFile: read template.json + thumb.png
ZipTemplateReader->>TemplateProviderImpl: construct ProjectTemplate (baseZipProject)
end
ZipTemplateReader->>ZipFile: close
end
Client->>ZipRecipeExecutor: execute(executor) (when creating project)
ZipRecipeExecutor->>ZipFile: open archive
ZipRecipeExecutor->>PebbleEngine: render .peb templates with params
loop per entry
ZipRecipeExecutor->>ZipRecipeExecutor: shouldSkipFile(language)?
alt peb template
PebbleEngine-->>ZipRecipeExecutor: rendered content
else raw file
ZipRecipeExecutor-->>ZipRecipeExecutor: copy bytes
end
ZipRecipeExecutor->>FileSystem: write file to project dir
end
ZipRecipeExecutor->>ZipFile: close
ZipRecipeExecutor->>Client: return ProjectTemplateRecipeResult
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
- ADFA-2179 ndk template #882: Overlaps TemplateProviderImpl changes (archive-driven template loading and template constructor changes).
- ADFA-2498 Plumbing for plugin-artifacts.zip #792: Related asset bundling/installation changes touching installers and archive assets.
- ADFA-1304 | Add Project Info Bottom Sheet #706: Overlaps TemplateDetailsFragment name/language handling changes.
Suggested reviewers
- itsaky-adfa
- jatezzz
- Daniel-ADFA
Poem
🐰 I hopped through zips with whiskers bright,
Pebble sparks and thumbnails in sight.
I unwrap templates, stitch params with care,
Projects spring from archives, tidy and fair.
🥕✨
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 12.12% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title 'next gen template initial version' directly describes the main change in the pull request. |
| Description check | ✅ Passed | The description 'initial next gen template' is related to the changeset and refers to the main feature being introduced. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
- 📝 Generate docstrings (stacked PR)
- 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
feat/ADFA-2737-template-initial
📝 Coding Plan
- Generate coding plan for human review comments
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 help to get the list of available commands and usage tips.
![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: 9
🧹 Nitpick comments (6)
templates-impl/src/main/java/com/itsaky/androidide/templates/impl/TemplateProviderImpl.kt (2)
54-54: Remove commented-out code.The commented line
// val folder = File("/sdcard/Download/templates")should be removed before merging.♻️ Cleanup
private fun initializeTemplates() { - // val folder = File("/sdcard/Download/templates") val folder = TEMPLATES_DIR🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/TemplateProviderImpl.kt` at line 54, Remove the leftover commented-out declaration in TemplateProviderImpl.kt: delete the line "// val folder = File("/sdcard/Download/templates")" (the unused commented variable `folder`) so the class TemplateProviderImpl no longer contains stray commented code before merging.
90-95: Remove commented-out coroutine code or implement async loading.The commented-out
CoroutineScope(Dispatchers.IO).launchsuggests async loading was considered. Either implement it or remove the commented code. Note thatinitializeTemplates()currently runs synchronously, which could block if called on the main thread.♻️ Cleanup commented code
override fun reload() { release() - // CoroutineScope(Dispatchers.IO).launch { initializeTemplates() - // } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/TemplateProviderImpl.kt` around lines 90 - 95, The reload() method in TemplateProviderImpl contains commented-out coroutine code around initializeTemplates(), so either remove the commented CoroutineScope(Dispatchers.IO).launch lines to clean up synchronous behavior or actually perform async loading by running initializeTemplates() inside a CoroutineScope(Dispatchers.IO).launch (or a passed-in scope), ensuring you keep release() semantics, catch and log exceptions from initializeTemplates(), and marshal any UI/thread-bound callbacks back to the main dispatcher; refer to TemplateProviderImpl.reload(), initializeTemplates(), CoroutineScope(Dispatchers.IO).launch and Dispatchers.IO when making the change.templates-api/src/main/java/com/itsaky/androidide/templates/base/ProjectTemplateBuilder.kt (1)
194-204: Non-null assertions follow existing pattern but carry crash risk.The
!!operators ontemplateNameStrandthumbDataare consistent with the existing pattern fortemplateName!!,thumb!!,widgets!!, andrecipe!!. However, this means if a caller fails to set these properties before callingbuildInternal(), aNullPointerExceptionwill be thrown at runtime.Consider whether more graceful error handling (e.g.,
checkNotNull()with a descriptive message) would improve debuggability, consistent with howdefModuleis validated at line 56.♻️ Optional improvement for better error messages
override fun buildInternal(): ProjectTemplate = ProjectTemplate( modules, - templateName!!, - thumb!!, + checkNotNull(templateName) { "templateName must be set before building" }, + checkNotNull(thumb) { "thumb must be set before building" }, tooltipTag, - widgets!!, - recipe!!, - templateNameStr!!, - thumbData!! + checkNotNull(widgets) { "widgets must be set before building" }, + checkNotNull(recipe) { "recipe must be set before building" }, + checkNotNull(templateNameStr) { "templateNameStr must be set before building" }, + checkNotNull(thumbData) { "thumbData must be set before building" } )🤖 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/ProjectTemplateBuilder.kt` around lines 194 - 204, Replace direct non-null assertions in ProjectTemplateBuilder.buildInternal() with explicit checks using checkNotNull (or requireNotNull) that include descriptive messages; specifically validate templateNameStr and thumbData (and optionally templateName, thumb, widgets, recipe) before constructing ProjectTemplate so failures produce clear error messages similar to the defModule validation pattern, referencing buildInternal(), ProjectTemplateBuilder, and the properties templateNameStr and thumbData to locate where to add the checks.app/src/main/java/com/itsaky/androidide/adapters/TemplateListAdapter.kt (1)
78-82: Remove or reduce verbose debug logging before merging.The debug logs at lines 78-80 and 82 are useful during development but may be excessive for production. They're called for every item in the RecyclerView, potentially logging large amounts of data.
♻️ Suggested cleanup
- log.debug("template: $template") templateName.text = template.templateNameStr - log.debug("text: ${template.templateNameStr} templateName.text: ${templateName.text}") if (template.thumbData != null) { - log.debug("thumbData is not null") Glide.with(templateIcon.context) .asBitmap() .load(template.thumbData) .into(templateIcon) } else { templateIcon.setImageResource(template.thumb) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/java/com/itsaky/androidide/adapters/TemplateListAdapter.kt` around lines 78 - 82, In TemplateListAdapter (inside the RecyclerView binding code, e.g., onBindViewHolder or bind method) remove or reduce the three per-item debug statements (the log.debug calls that print "template: $template", "text: ${template.templateNameStr} ..." and "thumbData is not null"); either delete them for production or wrap them behind a debug-level guard (e.g., log.isDebugEnabled) or replace with a single lightweight log that avoids printing full objects, and leave a single conditional debug for thumbData if absolutely needed.templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipTemplateReader.kt (2)
60-64: Remove commented-out code before merging.This commented block appears to be dead code. If the widget parsing logic is needed for future work, consider tracking it in an issue rather than leaving commented code in the codebase.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipTemplateReader.kt` around lines 60 - 64, Remove the commented-out widget parsing block to clean up dead code: delete the commented lines that define the widgets variable alternatives (the lines referencing widgetsEntry, zip.getInputStream, parseWidgetsFromJson(...) and the fallback metaJson.widgets.mapNotNull { it.toWidget() }) so only the active widget-parsing implementation remains; if this alternative logic must be preserved for future reference, create an issue or add a brief note in the commit message instead of leaving commented code in ZipTemplateReader.kt.
86-92: Prefer narrow exception handling over broadcatch (e: Exception).Using a broad
catch (e: Exception)masks specific failure modes and can hide bugs. Per project learnings, catch only the specific exception types expected (e.g.,IOException,JsonSyntaxException) and let unexpected exceptions propagate for fail-fast behavior during development.♻️ Proposed narrower exception handling
- } catch (e: Exception) { + } catch (e: IOException) { + log.error("Failed to load template at ${templateRef.path}", e) + } catch (e: com.google.gson.JsonSyntaxException) { log.error("Failed to load template at ${templateRef.path}", e) } } - } catch (e: Exception) { + } catch (e: IOException) { + log.error("Failed to read zip file $zipFile", e) + } catch (e: java.util.zip.ZipException) { log.error("Failed to read zip file $zipFile", e) - // return emptyList() }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 `@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipTemplateReader.kt` around lines 86 - 92, Replace the broad catch blocks in ZipTemplateReader with narrow, specific exception handlers: for the inner block that logs "Failed to load template at ${templateRef.path}" catch expected parsing/validation exceptions (e.g., JsonSyntaxException, IllegalArgumentException) and IO-related exceptions separately and log them; for the outer block that logs "Failed to read zip file $zipFile" catch IOExceptions (and ZipException if applicable) only and let other unexpected exceptions propagate. Update the catch clauses around the code that reads/parses entries (references: ZipTemplateReader, the inner template-loading try block and the outer zip-reading try block) to use multiple specific catches rather than catch (e: Exception).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@app/src/main/java/com/itsaky/androidide/fragments/TemplateDetailsFragment.kt`:
- Around line 139-142: Replace the hardcoded "unknown" fallback with a string
resource: add <string name="language_unknown">Unknown</string> to strings.xml
and update the assignment in TemplateDetailsFragment (where language is set from
result.data.language?.name) to use getString(R.string.language_unknown) (or
requireContext().getString(...)) as the default; this preserves i18n and keeps
the nullable-safe fallback intact.
In
`@composite-builds/build-deps-common/constants/src/main/java/org/adfa/constants/constants.kt`:
- Around line 81-82: The constant TEMPLATE_CORE_ARCHIVE is incorrectly
concatenated as "core$TEMPLATE_ARCHIVE_EXTENSION" which yields "corecgt" instead
of "core.cgt"; update the concatenation for TEMPLATE_CORE_ARCHIVE to include the
missing dot (e.g., "core.$TEMPLATE_ARCHIVE_EXTENSION" or equivalent) so it
matches File.extension behavior and produces "core.cgt".
In `@templates-api/src/main/java/com/itsaky/androidide/templates/template.kt`:
- Around line 344-348: The new requireNotNull(templateNameStr) in
Template<R>.build() causes templates constructed via
baseProject()/baseProjectImpl() (e.g., emptyActivityProject(),
noActivityProjectTemplate()) to throw because they never set templateNameStr;
either remove/relax the require in build() so templateNameStr is optional at
build time, or update all baseProject*/template builders to set templateNameStr
before calling build(); locate the check in build() (template.kt) and choose one
fix: (A) delete or replace requireNotNull(templateNameStr) with an optional
fallback/nullable handling, or (B) ensure baseProject(), baseProjectImpl(),
emptyActivityProject(), noActivityProjectTemplate() set templateNameStr (similar
to ZipTemplateReader which assigns this.templateNameStr = metaJson.name) prior
to build().
In
`@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/TemplateProviderImpl.kt`:
- Around line 76-78: Replace the broad catch (e: Throwable) {
e.printStackTrace() } in TemplateProviderImpl with a narrow catch for the
specific exception(s) observed in crashes (e.g., catch (e:
IllegalArgumentException)) and use the class SLF4J logger to record the error
(e.g., logger.error("Descriptive message about what failed", e)); if multiple
specific exceptions can occur, add separate catches for them rather than
catching Throwable or Exception.
In
`@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipJson.kt`:
- Around line 84-107: The toWidget() implementation currently calls
StringParameterBuilder.build() and BooleanParameterBuilder.build() without
providing required fields (name and for string default), causing NPEs; guard
these calls by validating WidgetJson.name (and WidgetJson.default for "string")
before constructing builders—if name is null/blank or string default is null,
log a warning and return null instead of building; update the "string" and
"boolean" branches in toWidget() to perform these checks prior to using
StringParameterBuilder and BooleanParameterBuilder so build() is never called
with missing required values.
In
`@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipRecipeExecutor.kt`:
- Around line 44-48: The ZipFile returned by zipProvider() (variable zip in
ZipRecipeExecutor.kt) is never closed; wrap the zipProvider() call so the
ZipFile is closed after use (e.g., use Kotlin's .use { zip -> ... } or a
try/finally that calls zip.close()), moving the early-return logic that returns
ProjectTemplateRecipeResultImpl(data) inside the resource block so the zip is
closed in all code paths before returning.
- Around line 93-98: Pebble template parsing/evaluation in the ZipRecipeExecutor
(calls to pebbleEngine.getTemplate(content) and template.evaluate(writer,
params) while processing entry.name) can throw PebbleException and currently
will abort ZIP processing; wrap the template retrieval and evaluation for
TEMPLATE_EXTENSION entries in a try/catch that catches PebbleException (and
optionally Exception) so a single bad template is logged via log.error including
entry.name and the exception, and then skip writing the outFile for that entry
(or write a safe fallback) and continue processing the rest of the ZIP; ensure
you still close any writers/streams (StringWriter) in finally or use Kotlin
use/try-with-resources patterns so resources are not leaked.
- Around line 89-103: The current code reads the entry content once into content
via zip.getInputStream(entry).bufferedReader().readText() but then re-opens
streams and creates a FileOutputStream even for template files
(TEMPLATE_EXTENSION) where outFile.writeText(...) is used instead of the opened
output, causing redundant IO and brief file locks; fix by branching on
entry.name.endsWith(TEMPLATE_EXTENSION) before opening the input/output streams:
for template files use the already-read content with
pebbleEngine.getTemplate(content) / template.evaluate(...) and write via
outFile.writeText(...); for non-template files skip reading into content and
instead open zip.getInputStream(entry).use { input ->
FileOutputStream(outFile).use { output -> input.copyTo(output) } } so
FileOutputStream is only created when actually used and streams are not
duplicated.
In
`@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipTemplateReader.kt`:
- Around line 29-31: ZipFile opened in ZipTemplateReader (the local variable
zip) is never closed; wrap ZipFile(zipFile) in a Kotlin use { } block or
otherwise ensure zip.close() is called (e.g., try-with-resources style) inside
the method that constructs it so the ZipFile is closed on all code paths and
avoids leaking file handles.
---
Nitpick comments:
In `@app/src/main/java/com/itsaky/androidide/adapters/TemplateListAdapter.kt`:
- Around line 78-82: In TemplateListAdapter (inside the RecyclerView binding
code, e.g., onBindViewHolder or bind method) remove or reduce the three per-item
debug statements (the log.debug calls that print "template: $template", "text:
${template.templateNameStr} ..." and "thumbData is not null"); either delete
them for production or wrap them behind a debug-level guard (e.g.,
log.isDebugEnabled) or replace with a single lightweight log that avoids
printing full objects, and leave a single conditional debug for thumbData if
absolutely needed.
In
`@templates-api/src/main/java/com/itsaky/androidide/templates/base/ProjectTemplateBuilder.kt`:
- Around line 194-204: Replace direct non-null assertions in
ProjectTemplateBuilder.buildInternal() with explicit checks using checkNotNull
(or requireNotNull) that include descriptive messages; specifically validate
templateNameStr and thumbData (and optionally templateName, thumb, widgets,
recipe) before constructing ProjectTemplate so failures produce clear error
messages similar to the defModule validation pattern, referencing
buildInternal(), ProjectTemplateBuilder, and the properties templateNameStr and
thumbData to locate where to add the checks.
In
`@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/TemplateProviderImpl.kt`:
- Line 54: Remove the leftover commented-out declaration in
TemplateProviderImpl.kt: delete the line "// val folder =
File("/sdcard/Download/templates")" (the unused commented variable `folder`) so
the class TemplateProviderImpl no longer contains stray commented code before
merging.
- Around line 90-95: The reload() method in TemplateProviderImpl contains
commented-out coroutine code around initializeTemplates(), so either remove the
commented CoroutineScope(Dispatchers.IO).launch lines to clean up synchronous
behavior or actually perform async loading by running initializeTemplates()
inside a CoroutineScope(Dispatchers.IO).launch (or a passed-in scope), ensuring
you keep release() semantics, catch and log exceptions from
initializeTemplates(), and marshal any UI/thread-bound callbacks back to the
main dispatcher; refer to TemplateProviderImpl.reload(), initializeTemplates(),
CoroutineScope(Dispatchers.IO).launch and Dispatchers.IO when making the change.
In
`@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipTemplateReader.kt`:
- Around line 60-64: Remove the commented-out widget parsing block to clean up
dead code: delete the commented lines that define the widgets variable
alternatives (the lines referencing widgetsEntry, zip.getInputStream,
parseWidgetsFromJson(...) and the fallback metaJson.widgets.mapNotNull {
it.toWidget() }) so only the active widget-parsing implementation remains; if
this alternative logic must be preserved for future reference, create an issue
or add a brief note in the commit message instead of leaving commented code in
ZipTemplateReader.kt.
- Around line 86-92: Replace the broad catch blocks in ZipTemplateReader with
narrow, specific exception handlers: for the inner block that logs "Failed to
load template at ${templateRef.path}" catch expected parsing/validation
exceptions (e.g., JsonSyntaxException, IllegalArgumentException) and IO-related
exceptions separately and log them; for the outer block that logs "Failed to
read zip file $zipFile" catch IOExceptions (and ZipException if applicable) only
and let other unexpected exceptions propagate. Update the catch clauses around
the code that reads/parses entries (references: ZipTemplateReader, the inner
template-loading try block and the outer zip-reading try block) to use multiple
specific catches rather than catch (e: Exception).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1c8437c1-522d-4e39-b8ee-cf69e7019cb9
📒 Files selected for processing (16)
app/src/main/java/com/itsaky/androidide/adapters/TemplateListAdapter.ktapp/src/main/java/com/itsaky/androidide/fragments/TemplateDetailsFragment.ktapp/src/main/java/com/itsaky/androidide/fragments/TemplateListFragment.ktcommon/src/main/java/com/itsaky/androidide/utils/Environment.javacomposite-builds/build-deps-common/constants/src/main/java/org/adfa/constants/constants.ktgradle/libs.versions.tomltemplates-api/build.gradle.ktstemplates-api/src/main/java/com/itsaky/androidide/templates/base/ProjectTemplateBuilder.kttemplates-api/src/main/java/com/itsaky/androidide/templates/base/base.kttemplates-api/src/main/java/com/itsaky/androidide/templates/template.kttemplates-impl/build.gradle.ktstemplates-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/ZipRecipeExecutor.kttemplates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipTemplateConstants.kttemplates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipTemplateReader.kt
Comment on lines
+139
to
+142
| // templateName = getString(template.templateName), | ||
| templateName = template.templateNameStr, | ||
| // language = result.data.language.name | ||
| language = result.data.language?.name ?: "unknown" |
Contributor
There was a problem hiding this comment.
Consider using a string resource for the "unknown" fallback.
The changes correctly adapt to the new nullable language property and templateNameStr field. However, the hardcoded "unknown" string bypasses internationalization.
💡 Suggested improvement
- language = result.data.language?.name ?: "unknown"
+ language = result.data.language?.name ?: getString(R.string.language_unknown)Add the string resource in strings.xml:
<string name="language_unknown">Unknown</string>📝 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
| // templateName = getString(template.templateName), | |
| templateName = template.templateNameStr, | |
| // language = result.data.language.name | |
| language = result.data.language?.name ?: "unknown" | |
| // templateName = getString(template.templateName), | |
| templateName = template.templateNameStr, | |
| // language = result.data.language.name | |
| language = result.data.language?.name ?: getString(R.string.language_unknown) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/java/com/itsaky/androidide/fragments/TemplateDetailsFragment.kt`
around lines 139 - 142, Replace the hardcoded "unknown" fallback with a string
resource: add <string name="language_unknown">Unknown</string> to strings.xml
and update the assignment in TemplateDetailsFragment (where language is set from
result.data.language?.name) to use getString(R.string.language_unknown) (or
requireContext().getString(...)) as the default; this preserves i18n and keeps
the nullable-safe fallback intact.
composite-builds/build-deps-common/constants/src/main/java/org/adfa/constants/constants.kt
Outdated
Show resolved
Hide resolved
Comment on lines
344
to
+348
| fun build(): Template<R> { | ||
| requireNotNull(templateName) { "Template must have a name" } | ||
| requireNotNull(templateName) { "Template must have a name id" } | ||
| requireNotNull(thumb) { "Template must have a thumbnail" } | ||
| requireNotNull(recipe) { "Template must have a recipe" } | ||
| requireNotNull(templateNameStr) {"Template must have a name"} |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if any existing templates set templateNameStr before build() is called
rg -n 'templateNameStr\s*=' --type=kt -g '!**/ZipTemplateReader.kt'Repository: appdevforall/CodeOnTheGo
Length of output: 94
🏁 Script executed:
#!/bin/bash
# Find kotlin files and search for templateNameStr assignments
fd -e kt | head -20Repository: appdevforall/CodeOnTheGo
Length of output: 1790
🏁 Script executed:
#!/bin/bash
# Search for templateNameStr assignments across all files
rg 'templateNameStr\s*=' -A 2Repository: appdevforall/CodeOnTheGo
Length of output: 425
🏁 Script executed:
#!/bin/bash
# Look at the template.kt file specifically, lines 344-348
fd -e kt -p template.kt | xargs -I {} cat -n {} | sed -n '340,350p'Repository: appdevforall/CodeOnTheGo
Length of output: 489
🏁 Script executed:
#!/bin/bash
# Find baseProject function definition
rg 'fun baseProject' -A 20Repository: appdevforall/CodeOnTheGo
Length of output: 4901
🏁 Script executed:
#!/bin/bash
# Check if baseProject() returns build() or calls it
rg 'build\(\)' -A 2 -B 2 templates-api/src/main/java/com/itsaky/androidide/templates/base/base.ktRepository: appdevforall/CodeOnTheGo
Length of output: 402
🏁 Script executed:
#!/bin/bash
# Check the emptyActivityTemplate.kt to see if it sets templateNameStr
cat -n templates-impl/src/main/java/com/itsaky/androidide/templates/impl/emptyActivity/emptyActivityTemplate.kt | head -50Repository: appdevforall/CodeOnTheGo
Length of output: 2293
🏁 Script executed:
#!/bin/bash
# Check noActivityTemplate.kt as well
cat -n templates-impl/src/main/java/com/itsaky/androidide/templates/impl/noActivity/noActivityTemplate.kt | head -50Repository: appdevforall/CodeOnTheGo
Length of output: 1742
🏁 Script executed:
#!/bin/bash
# Search for all places where baseProject is called
rg 'baseProject\(' --type-list | head -5Repository: appdevforall/CodeOnTheGo
Length of output: 457
🏁 Script executed:
#!/bin/bash
# Search for uses of baseProject()
rg 'baseProject\(' -B 5 -A 10Repository: appdevforall/CodeOnTheGo
Length of output: 3526
Breaking change: templateNameStr is now required but baseProject() doesn't set it.
Line 348 adds requireNotNull(templateNameStr), but templates using baseProject() (such as emptyActivityProject() and noActivityProjectTemplate()) never set templateNameStr before build() is called. This will cause all such templates to fail at runtime with a NullPointerException.
Only ZipTemplateReader sets templateNameStr (line: this.templateNameStr = metaJson.name). All other template implementations built with baseProject() or baseProjectImpl() will break.
Fix: Make templateNameStr optional in the build() method, or ensure all template builders that call build() are updated to set templateNameStr.
🤖 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/template.kt`
around lines 344 - 348, The new requireNotNull(templateNameStr) in
Template<R>.build() causes templates constructed via
baseProject()/baseProjectImpl() (e.g., emptyActivityProject(),
noActivityProjectTemplate()) to throw because they never set templateNameStr;
either remove/relax the require in build() so templateNameStr is optional at
build time, or update all baseProject*/template builders to set templateNameStr
before calling build(); locate the check in build() (template.kt) and choose one
fix: (A) delete or replace requireNotNull(templateNameStr) with an optional
fallback/nullable handling, or (B) ensure baseProject(), baseProjectImpl(),
emptyActivityProject(), noActivityProjectTemplate() set templateNameStr (similar
to ZipTemplateReader which assigns this.templateNameStr = metaJson.name) prior
to build().
templates-impl/src/main/java/com/itsaky/androidide/templates/impl/TemplateProviderImpl.kt
Outdated
Show resolved
Hide resolved
templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipJson.kt
Outdated
Show resolved
Hide resolved
templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipRecipeExecutor.kt
Outdated
Show resolved
Hide resolved
templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipRecipeExecutor.kt
Outdated
Show resolved
Hide resolved
Comment on lines
+93
to
+98
| if (entry.name.endsWith(TEMPLATE_EXTENSION)) { | ||
| log.debug("template processing ${entry.name}") | ||
| val template = pebbleEngine.getTemplate(content) | ||
| val writer = StringWriter() | ||
| template.evaluate(writer, params) | ||
| outFile.writeText(writer.toString(), Charsets.UTF_8) |
Contributor
There was a problem hiding this comment.
Pebble template evaluation can throw exceptions that aren't handled.
pebbleEngine.getTemplate() and template.evaluate() can throw PebbleException (or subclasses) if the template has syntax errors or references undefined variables. Without handling, a single malformed template will abort the entire ZIP processing.
🛡️ Proposed defensive handling
if (entry.name.endsWith(TEMPLATE_EXTENSION)) {
log.debug("template processing ${entry.name}")
+ try {
val template = pebbleEngine.getTemplate(content)
val writer = StringWriter()
template.evaluate(writer, params)
outFile.writeText(writer.toString(), Charsets.UTF_8)
+ } catch (e: io.pebbletemplates.pebble.error.PebbleException) {
+ log.error("Failed to process template ${entry.name}", e)
+ // Optionally: copy the raw file or skip
+ }
} else {📝 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 (entry.name.endsWith(TEMPLATE_EXTENSION)) { | |
| log.debug("template processing ${entry.name}") | |
| val template = pebbleEngine.getTemplate(content) | |
| val writer = StringWriter() | |
| template.evaluate(writer, params) | |
| outFile.writeText(writer.toString(), Charsets.UTF_8) | |
| if (entry.name.endsWith(TEMPLATE_EXTENSION)) { | |
| log.debug("template processing ${entry.name}") | |
| try { | |
| val template = pebbleEngine.getTemplate(content) | |
| val writer = StringWriter() | |
| template.evaluate(writer, params) | |
| outFile.writeText(writer.toString(), Charsets.UTF_8) | |
| } catch (e: io.pebbletemplates.pebble.error.PebbleException) { | |
| log.error("Failed to process template ${entry.name}", e) | |
| // Optionally: copy the raw file or skip | |
| } | |
| } else { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipRecipeExecutor.kt`
around lines 93 - 98, Pebble template parsing/evaluation in the
ZipRecipeExecutor (calls to pebbleEngine.getTemplate(content) and
template.evaluate(writer, params) while processing entry.name) can throw
PebbleException and currently will abort ZIP processing; wrap the template
retrieval and evaluation for TEMPLATE_EXTENSION entries in a try/catch that
catches PebbleException (and optionally Exception) so a single bad template is
logged via log.error including entry.name and the exception, and then skip
writing the outFile for that entry (or write a safe fallback) and continue
processing the rest of the ZIP; ensure you still close any writers/streams
(StringWriter) in finally or use Kotlin use/try-with-resources patterns so
resources are not leaked.
templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipTemplateReader.kt
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@templates-api/src/main/java/com/itsaky/androidide/templates/base/modules/android/buildGradle.kt`:
- Line 100: The template currently interpolates data.versions.minSdk?.api
directly (e.g. the string "minSdk = ${data.versions.minSdk?.api}") which can
emit "minSdk = null"; change the template generation so the entire "minSdk =
..." line is only emitted when data.versions.minSdk?.api is non-null (use a
conditional/let to omit the line if null) and apply this fix to all four
occurrences in buildGradle.kt (the KTS and Groovy template generators and their
NDK variants where data.versions.minSdk?.api is used).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 19f0d752-60b8-48c1-a5e7-bdc05c91538b
📒 Files selected for processing (1)
templates-api/src/main/java/com/itsaky/androidide/templates/base/modules/android/buildGradle.kt
templates-api/src/main/java/com/itsaky/androidide/templates/base/modules/android/buildGradle.kt
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/main/java/com/itsaky/androidide/assets/BundledAssetsInstaller.kt`:
- Around line 83-91: The new TEMPLATE_CORE_ARCHIVE case copies the bundled
archive but BundledAssetsInstaller.expectedSize() still returns 0 for
TEMPLATE_CORE_ARCHIVE, causing progress misreporting; update
BundledAssetsInstaller.expectedSize() to return the real expected size for
TEMPLATE_CORE_ARCHIVE (matching how other assets are handled) by computing or
returning the compressed or uncompressed byte length used by the installer so
AssetsInstallationHelper.totalSize includes TEMPLATE_CORE_ARCHIVE; locate the
expectedSize() method and add a branch for TEMPLATE_CORE_ARCHIVE (referencing
TEMPLATE_CORE_ARCHIVE constant) that returns the correct long value (or derives
it from the same source ToolsManager.getCommonAsset / asset metadata) to ensure
progress reflects the actual copy work.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6155d922-1073-4c2e-be78-7f8dfd09e922
📒 Files selected for processing (5)
app/build.gradle.ktsapp/src/main/java/com/itsaky/androidide/assets/AssetsInstallationHelper.ktapp/src/main/java/com/itsaky/androidide/assets/BundledAssetsInstaller.ktapp/src/main/java/com/itsaky/androidide/assets/SplitAssetsInstaller.ktcomposite-builds/build-deps-common/constants/src/main/java/org/adfa/constants/constants.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- composite-builds/build-deps-common/constants/src/main/java/org/adfa/constants/constants.kt
app/src/main/java/com/itsaky/androidide/assets/BundledAssetsInstaller.kt
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipRecipeExecutor.kt (1)
94-108:⚠️ Potential issue | 🟠 MajorRemove redundant stream work and ensure every opened stream is closed.
Line 94 reads the whole entry (as text) before branching, and that stream is not managed with
use. Also,FileOutputStreamis opened for template entries where it is never used.Proposed fix
- val content = zip.getInputStream(entry).bufferedReader().readText() - - zip.getInputStream(entry).use { input -> - FileOutputStream(outFile).use { output -> - if (entry.name.endsWith(TEMPLATE_EXTENSION)) { - log.debug("template processing ${entry.name}") - val template = pebbleEngine.getTemplate(content) - val writer = StringWriter() - template.evaluate(writer, params) - outFile.writeText(writer.toString(), Charsets.UTF_8) - } else { - input.copyTo(output) - } - } - } + if (entry.name.endsWith(TEMPLATE_EXTENSION)) { + log.debug("template processing ${entry.name}") + val content = zip.getInputStream(entry).bufferedReader().use { it.readText() } + val template = pebbleEngine.getTemplate(content) + val writer = StringWriter() + template.evaluate(writer, params) + outFile.writeText(writer.toString(), Charsets.UTF_8) + } else { + zip.getInputStream(entry).use { input -> + FileOutputStream(outFile).use { output -> + input.copyTo(output) + } + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipRecipeExecutor.kt` around lines 94 - 108, The code reads the entry into val content via zip.getInputStream(entry).bufferedReader().readText() without using use(), then later opens zip.getInputStream(entry) again and always creates a FileOutputStream(outFile) even for template entries; fix ZipRecipeExecutor by removing the initial readText call, only open the entry InputStream inside a use block when copying binary entries, and for TEMPLATE_EXTENSION entries read the entry via a buffered reader in a use block (or obtain the entry content via a single use-managed stream) and write the rendered template with outFile.writeText (creating FileOutputStream only when actually copying binary data), ensuring every InputStream/FileOutputStream/Reader/Writer is wrapped with use so all streams are closed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipRecipeExecutor.kt`:
- Around line 85-93: The code constructs output paths from ZipEntry names
(relativePath, outFile) without validating them, allowing zip-slip path
traversal; fix by resolving the target path against projectDir and verifying the
canonical path is inside projectDir before creating/writing: after computing
outFile (from relativePath.removeSuffix(TEMPLATE_EXTENSION)), call canonicalFile
on both projectDir and outFile and ensure outFile.canonicalPath
startsWith(projectDir.canonicalPath + File.separator) (or equals
projectDir.canonicalPath) and if not, skip or throw; only then create parent
dirs (outFile.parentFile?.mkdirs()) and write the file, keeping the existing
handling for directories (entry.isDirectory) intact and apply the same
validation for directory entries.
---
Duplicate comments:
In
`@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipRecipeExecutor.kt`:
- Around line 94-108: The code reads the entry into val content via
zip.getInputStream(entry).bufferedReader().readText() without using use(), then
later opens zip.getInputStream(entry) again and always creates a
FileOutputStream(outFile) even for template entries; fix ZipRecipeExecutor by
removing the initial readText call, only open the entry InputStream inside a use
block when copying binary entries, and for TEMPLATE_EXTENSION entries read the
entry via a buffered reader in a use block (or obtain the entry content via a
single use-managed stream) and write the rendered template with
outFile.writeText (creating FileOutputStream only when actually copying binary
data), ensuring every InputStream/FileOutputStream/Reader/Writer is wrapped with
use so all streams are closed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4713170a-32e7-4bb4-af9f-466c0a3bb638
📒 Files selected for processing (4)
templates-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/ZipRecipeExecutor.kttemplates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipTemplateReader.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipTemplateReader.kt
templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipRecipeExecutor.kt
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipRecipeExecutor.kt (1)
150-154: Consider making helper methods private to reduce API surface.These utilities appear internal to
ZipRecipeExecutorand don’t need to be publicly exposed.Also applies to: 156-159, 226-234
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipRecipeExecutor.kt` around lines 150 - 154, Make the utility helpers non-public: change safeLanguageName and safeMinSdkApi to private functions (e.g., private fun safeLanguageName(...) and private fun safeMinSdkApi(...)) and likewise change the other helper methods in the referenced ranges (startLine 156-159 and 226-234) to private; ensure you update any internal references within ZipRecipeExecutor to match the new visibility and if any external code depends on them, move those call sites into the class or provide a controlled public facade instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipRecipeExecutor.kt`:
- Around line 43-48: Check data.projectDir.exists() before calling zipProvider()
so archive IO is not touched when we can short-circuit; move the projectDir
existence check to occur prior to invoking zipProvider() in ZipRecipeExecutor
(returning ProjectTemplateRecipeResultImpl(data) when it exists) to avoid
opening the ZIP unnecessarily.
In
`@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipTemplateReader.kt`:
- Around line 101-103: In ZipTemplateReader, replace the broad catch blocks that
currently catch Exception with specific exception types: for the inner catch
that logs "Failed to load template at ${templateRef.path}" catch
java.io.IOException and com.google.gson.JsonParseException only (log the error
as before); for the outer catch around the ZIP/index opening logic catch
java.util.zip.ZipException, java.io.IOException and
com.google.gson.JsonParseException only; remove the generic catch so unexpected
exceptions propagate (and add the necessary imports).
---
Nitpick comments:
In
`@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipRecipeExecutor.kt`:
- Around line 150-154: Make the utility helpers non-public: change
safeLanguageName and safeMinSdkApi to private functions (e.g., private fun
safeLanguageName(...) and private fun safeMinSdkApi(...)) and likewise change
the other helper methods in the referenced ranges (startLine 156-159 and
226-234) to private; ensure you update any internal references within
ZipRecipeExecutor to match the new visibility and if any external code depends
on them, move those call sites into the class or provide a controlled public
facade instead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5d5e865c-daee-4476-9acb-56fe7bdc0781
📒 Files selected for processing (4)
app/build.gradle.ktsapp/src/main/java/com/itsaky/androidide/assets/BundledAssetsInstaller.kttemplates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipRecipeExecutor.kttemplates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipTemplateReader.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- app/build.gradle.kts
- app/src/main/java/com/itsaky/androidide/assets/BundledAssetsInstaller.kt
Comment on lines
+43
to
+48
| zipProvider().use { zip -> | ||
|
|
||
| val projectDir = data.projectDir | ||
| if (projectDir.exists()) { | ||
| return ProjectTemplateRecipeResultImpl(data) | ||
| } |
Contributor
There was a problem hiding this comment.
Check projectDir.exists() before opening the ZIP.
This path should short-circuit without touching archive IO. Right now zipProvider() can throw even when you intend to return immediately.
🔧 Suggested reorder
override fun execute(
executor: RecipeExecutor
): ProjectTemplateRecipeResult {
log.debug("executor called!!")
- zipProvider().use { zip ->
-
- val projectDir = data.projectDir
- if (projectDir.exists()) {
- return ProjectTemplateRecipeResultImpl(data)
- }
+ val projectDir = data.projectDir
+ if (projectDir.exists()) {
+ return ProjectTemplateRecipeResultImpl(data)
+ }
+ zipProvider().use { zip ->
val projectRoot = projectDir.canonicalFile🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipRecipeExecutor.kt`
around lines 43 - 48, Check data.projectDir.exists() before calling
zipProvider() so archive IO is not touched when we can short-circuit; move the
projectDir existence check to occur prior to invoking zipProvider() in
ZipRecipeExecutor (returning ProjectTemplateRecipeResultImpl(data) when it
exists) to avoid opening the ZIP unnecessarily.
Comment on lines
+101
to
+103
| } catch (e: Exception) { | ||
| log.error("Failed to load template at ${templateRef.path}", e) | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify broad catches in template ZIP loading paths.
rg -nP --type=kotlin -C2 'catch\s*\(\s*\w+\s*:\s*Exception\s*\)' \
templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipTemplateReader.kt \
templates-impl/src/main/java/com/itsaky/androidide/templates/impl/TemplateProviderImpl.ktRepository: appdevforall/CodeOnTheGo
Length of output: 1982
🏁 Script executed:
cat -n templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipTemplateReader.kt | sed -n '85,115p'Repository: appdevforall/CodeOnTheGo
Length of output: 1127
🏁 Script executed:
cat -n templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipTemplateReader.kt | sed -n '60,115p'Repository: appdevforall/CodeOnTheGo
Length of output: 2394
🏁 Script executed:
cat -n templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipTemplateReader.kt | sed -n '35,115p'Repository: appdevforall/CodeOnTheGo
Length of output: 3584
🏁 Script executed:
cat -n templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipTemplateReader.kt | sed -n '1,40p'Repository: appdevforall/CodeOnTheGo
Length of output: 1871
Replace broad catch (Exception) with expected exception types.
These catch-all blocks mask non-recoverable defects and can hide unexpected failures. Catch only the expected parse/IO exceptions and let unexpected failures surface for faster debugging.
For the inner catch at lines 101-103, expect exceptions from ZIP stream access and JSON parsing:
java.io.IOException(stream operations)com.google.gson.JsonParseException(metadata parsing)
For the outer catch at lines 106-107, expect exceptions from ZIP file operations and initial index parsing:
java.util.zip.ZipException(invalid zip archive)java.io.IOException(file access)com.google.gson.JsonParseException(index parsing)
🔧 Suggested narrowing pattern
- } catch (e: Exception) {
+ } catch (e: com.google.gson.JsonParseException) {
+ log.error("Failed to parse template metadata at ${templateRef.path}", e)
+ } catch (e: java.io.IOException) {
log.error("Failed to load template at ${templateRef.path}", e)
}
}
}
- } catch (e: Exception) {
+ } catch (e: java.util.zip.ZipException) {
+ log.error("Invalid zip archive $zipFile", e)
+ } catch (e: java.io.IOException) {
log.error("Failed to read zip file $zipFile", e)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@templates-impl/src/main/java/com/itsaky/androidide/templates/impl/zip/ZipTemplateReader.kt`
around lines 101 - 103, In ZipTemplateReader, replace the broad catch blocks
that currently catch Exception with specific exception types: for the inner
catch that logs "Failed to load template at ${templateRef.path}" catch
java.io.IOException and com.google.gson.JsonParseException only (log the error
as before); for the outer catch around the ZIP/index opening logic catch
java.util.zip.ZipException, java.io.IOException and
com.google.gson.JsonParseException only; remove the generic catch so unexpected
exceptions propagate (and add the necessary imports).
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
1 participant
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.
initial next gen template