ADFA-2693: Preview include tags

[dara-abijo-adfa]

Properly handle <include> tags when previewing layouts

[coderabbitai]

📝 Walkthrough

• Enhanced include tag support: preview now resolves tags by loading referenced layout files (using the layout's parent directory as basePath) so included content appears correctly in previews; placeholders are used when include targets are missing.
• Base-path propagation: layout loading computes and stores the current layout's parent path and propagates it to parsers (EditorActivity, PreviewLayoutActivity, DesignEditor) so relative include/resource paths are resolved correctly.
• Recursive include parsing: included XML files are converted and parsed recursively via XmlLayoutParser, supporting nested includes.
• Merge handling: parser recognizes tools:is_xml_merge and semantics; XmlLayoutGenerator emits proper blocks and short-circuits generation for merge wrappers.
• XmlParserUtils utilities: new helpers added for attribute extraction/application, include placeholders, and merge wrapper creation to centralize parsing tasks.
• Undo/redo and state handling: EditorActivity refreshes undo/redo history after loading; DesignEditor tracks currentBasePath, passes it into parsing, and avoids pushing empty editor states.
• Logging and error handling: XmlLayoutParser logs warnings/errors for missing or unreadable include files and handles parse failures more explicitly.
• Minor generator fix: XmlLayoutGenerator clears its internal buffer at the start of generate to avoid stale output between calls.

Risks / Best-practice notes:

• Breaking API changes: XmlLayoutParser constructor now accepts basePath (context, basePath?) and DesignEditor.loadLayoutFromParser(xml, basePath?) signature changed; update external callers and public API consumers.
• File I/O in parser: reading included files from disk introduces I/O failure modes—missing, unreadable, or malformed files can break previews; ensure callers surface failures gracefully and consider user-facing diagnostics.
• Recursive include/cycle risk: nested or circular includes may cause deep recursion or infinite loops—consider adding recursion depth limits or cycle detection.
• Placeholder behavior: when included layouts are missing, placeholders are injected (marked as includes); verify UX and downstream handling to avoid confusing previews or accidental commits.
• Increased complexity & testing surface: substantial parsing and generation logic added (XmlLayoutParser, XmlLayoutGenerator, XmlParserUtils) — requires thorough unit/integration tests covering includes, merges, attributes, edge cases, and performance.
• Dependency on conversion utility: included-file parsing relies on ConvertImportedXml; ensure it robustly handles expected XML variants and encoding/namespace edge cases.

Walkthrough

Propagates an optional basePath from activities through DesignEditor into XmlLayoutParser to resolve relative includes; XmlLayoutParser now loads and merges included XML files and recognizes/marks blocks; XmlLayoutGenerator emits elements during generation.

Changes

Cohort / File(s)	Summary
Activity Layer Integration layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/activities/EditorActivity.kt, layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/activities/PreviewLayoutActivity.kt	Compute basePath from layout file path and pass it into layout loading; EditorActivity refreshes undo/redo after load; PreviewActivity constructs parser with basePath.
Layout Editor Interface layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/DesignEditor.kt	loadLayoutFromParser signature extended to basePath: String? = null; stores currentBasePath, constructs XmlLayoutParser(context, basePath), and avoids pushing empty undo states.
Core Parser Enhancement layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/tools/XmlLayoutParser.kt	Constructor gains basePath: String?; supports loading/parsing included layouts from filesystem relative to basePath, applies include attributes, handles <merge> by wrapping and marking with tools:is_xml_merge, adds logging and helper methods.
Parser Utilities layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/tools/XMLParserUtils.kt	New utility object providing attribute extraction, getAttribute, applyAttributes, include placeholder creation, and merge wrapper creation used by XmlLayoutParser.
Layout Generator Update layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/tools/XmlLayoutGenerator.java	Added tryWriteMerge to emit <merge> when marker present; generate clears buffer upfront; peek pre-checks for merge to short-circuit rendering.

Sequence Diagram(s)

sequenceDiagram
participant Activity as EditorActivity/PreviewLayoutActivity
participant DesignEditor
participant Parser as XmlLayoutParser
participant FS as FileSystem
participant Conv as ConvertImportedXml
rect rgba(200,230,255,0.5)
Activity->>Activity: compute basePath from layout file path
Activity->>DesignEditor: loadLayoutFromParser(xml, basePath)
DesignEditor->>Parser: new XmlLayoutParser(context, basePath)
Parser->>Parser: parseFromXml(xml)
alt include tag present (layout attr)
Parser->>FS: read referenced XML (basePath + layout)
FS-->>Parser: file content / error
Parser->>Conv: ConvertImportedXml(file content)
Conv-->>Parser: converted XML
Parser->>Parser: parseFromXml(converted XML) (recursive)
Parser->>Parser: merge include attributes into parsed view
end
alt merge marker created
Parser->>Parser: wrap and mark with tools:is_xml_merge
end
Parser-->>DesignEditor: root view returned
DesignEditor-->>Activity: layout loaded (undo/redo updated)
end

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• ADFA-2655: Fix fragment tag being replaced with FrameLayout after layout editor save #914 — Modifies parser/generator handling around include/placeholder and special-case emission; likely overlaps with include/merge logic changes.

Suggested reviewers

• itsaky-adfa
• jomen-adfa
• jatezzz

Poem

🐰
I hopped through folders, sniffed each line,
Found hidden includes and stitched them fine,
With basePath in paw I stitched the vine,
Merged branches now gleam — layouts align,
A rabbit's nibble: code and carrot, divine! 🥕

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% 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 PR title 'ADFA-2693: Preview include tags' clearly and accurately summarizes the main objective of the changeset, which is to properly handle tags when previewing layouts.
Description check	✅ Passed	The PR description 'Properly handle <include> tags when previewing layouts' is directly related to the changeset and explains the core purpose of the modifications.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch ADFA-2693-preview-include-tags

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/tools/XmlLayoutParser.kt (1)

65-91: ⚠️ Potential issue | 🟠 Major

Avoid clearing global IdManager during nested include parsing.

parseFromXml() calls clear() unconditionally, so every included layout parse wipes IDs collected from earlier includes. With multiple <include> tags, only the last include’s IDs survive, leading to collisions and incorrect ID mapping.

Consider making ID reset/registration optional for nested parses and disabling it for includes.

🛠️ Suggested fix (skip ID reset/registration for nested parses)

-fun parseFromXml(-	xml: String,-	context: Context,-) {+fun parseFromXml(+	xml: String,+	context: Context,+	resetIds: Boolean = true,+) {
listViews.clear()
viewAttributeMap.clear()
-	clear()+	if (resetIds) clear()
...
-	for ((view, map) in viewAttributeMap) {- if (map.contains("android:id")) {- addNewId(view, map.getValue("android:id"))- }- applyAttributes(view, map)-	}+	for ((view, map) in viewAttributeMap) {+ if (resetIds && map.contains("android:id")) {+ addNewId(view, map.getValue("android:id"))+ }+ applyAttributes(view, map)+	}
}

layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/editor/DesignEditor.kt (1)

453-474: ⚠️ Potential issue | 🟠 Major

Preserve basePath for undo/redo reloads.

loadLayoutFromParser now accepts basePath, but undo/redo still call it with the default null. That breaks <include> resolution after undo/redo operations.

Cache the last base path and reuse it when loading from history.

🛠️ Suggested fix (cache basePath)

 class DesignEditor : LinearLayout {
+	private var lastBasePath: String? = null
...
-	fun loadLayoutFromParser(xml: String, basePath: String? = null) {+	fun loadLayoutFromParser(xml: String, basePath: String? = null) {+ lastBasePath = basePath
clearAll()
if (xml.isEmpty()) return
val parser = XmlLayoutParser(context, basePath)
...
}
...
fun undo() {
if (undoRedoManager == null) return
- if (undoRedoManager!!.isUndoEnabled) loadLayoutFromParser(undoRedoManager!!.undo())+ if (undoRedoManager!!.isUndoEnabled) loadLayoutFromParser(undoRedoManager!!.undo(), lastBasePath)
}
fun redo() {
if (undoRedoManager == null) return
- if (undoRedoManager!!.isRedoEnabled) loadLayoutFromParser(undoRedoManager!!.redo())+ if (undoRedoManager!!.isRedoEnabled) loadLayoutFromParser(undoRedoManager!!.redo(), lastBasePath)
}
}

🤖 Fix all issues with AI agents

In
`@layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/activities/EditorActivity.kt`:
- Around line 759-760: The restore path loses the basePath used for includes
because you pass parentPath only on initial load
(binding.editorLayout.loadLayoutFromParser(design, parentPath)) but later calls
in restoreOriginalXmlIfNeeded call loadLayoutFromParser without the basePath;
cache the resolved parentPath (e.g., store it on the Activity or EditorLayout
instance when you first compute parentPath) and reuse that cached basePath when
calling loadLayoutFromParser inside restoreOriginalXmlIfNeeded so <include>
resolution remains consistent after discard/restore.
In
`@layouteditor/src/main/java/org/appdevforall/codeonthego/layouteditor/tools/XmlLayoutParser.kt`:
- Around line 129-207: The included layout handling must merge the included
parser’s viewAttributeMap into the parent and correctly handle <merge> roots:
after creating and parsing includedParser (XmlLayoutParser.parseFromXml) iterate
includedParser.viewAttributeMap and for each entry add the corresponding child
View(s) from includedParser.root(s) into the parent listViews (or wrap multiple
roots in a synthetic container) and copy/merge their AttributeMap values into
the parent viewAttributeMap (preserving MARKER_IS_INCLUDE and any overridden
attributes from the <include> tag). Also detect when the included layout root
was a <merge> (or when includedParser exposes multiple roots) and in that case
add all parsed child Views instead of only includedParser.root, applying
attribute overrides per child; ensure the fallback placeholder branch still
merges attributes into the correct viewAttributeMap entries rather than losing
metadata.