ADFA-2106 | Enable text selection in build logs#732
Conversation
…ates log viewer to support native text selection and adds tooltip action to the selection toolbar
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
📝 WalkthroughRelease Notes: Enable Text Selection in Build Logs (ADFA-2106)Features
Technical Changes
|
| Cohort / File(s) | Summary |
|---|---|
Action ID & visibility app/src/main/java/com/itsaky/androidide/actions/file/ShowTooltipAction.kt |
Action ID changed from "ide.editor.code.text.format" to "ide.editor.code.text.show_tooltip"; added override fun prepare(data: ActionData) to hide/disable when no editor and to enable only when selection state indicates visibility. |
Fragment long-press API & dispatch app/src/main/java/com/itsaky/androidide/fragments/EmptyStateFragment.kt |
Added open val currentEditor: IDEEditor?; replaced single abstract onFragmentLongPressed() with open fun onFragmentLongPressed(x: Float = -1f, y: Float = -1f) plus a no-arg overload; long-press handling now forwards MotionEvent coordinates and short-circuits when editor is read-only. |
Fragments exposing editor & tooltip tag app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt, app/src/main/java/com/itsaky/androidide/fragments/output/LogViewFragment.kt |
Removed fragment-specific onFragmentLongPressed() overrides; added override val currentEditor: IDEEditor? get() = editor and set editor?.tag = TooltipTag.PROJECT_BUILD_OUTPUT / editor.tag = tooltipTag in view setup. |
Editor: selection from touch editor/src/main/java/com/itsaky/androidide/editor/ui/IDEEditor.kt |
Added fun setSelectionFromPoint(x: Float, y: Float) that maps touch coordinates to a packed position, extracts line/column, and calls setSelection(line, column) with error handling and early-return when released. |
Sequence Diagram
sequenceDiagram
participant User
participant GestureDetector
participant EmptyStateFragment
participant IDEEditor
Note over EmptyStateFragment: Fragment may expose currentEditor
User->>GestureDetector: long-press at (x,y)
GestureDetector->>EmptyStateFragment: onFragmentLongPressed(x,y)
alt currentEditor != null and not read-only
EmptyStateFragment->>IDEEditor: setSelectionFromPoint(x,y)
IDEEditor->>IDEEditor: map point -> packed position
IDEEditor->>IDEEditor: extract line,col and call setSelection
IDEEditor-->>EmptyStateFragment: selection updated
else no editor or read-only
EmptyStateFragment-->>GestureDetector: no-op / ignore
end
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Areas to focus on:
EmptyStateFragmentlong-press overloads and coordinate forwarding (default params, read-only short-circuit).IDEEditor.setSelectionFromPointcorrectness (point → packed position math, bounds, exception handling).ShowTooltipAction.preparevisibility/enablement logic and tag resolution in execAction.
Poem
🐰 A hop, a press, a pinpoint nudge,
I trace the line where characters judge.
Fragments pass the tangent true,
The editor hops and selects for you —
Tiny paws, precise little judge. 🥕✨
Pre-merge checks and finishing touches
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. | You can run @coderabbitai generate docstrings to improve docstring coverage. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly summarizes the main change: enabling text selection in build logs through modified long-press behavior. |
| Description check | ✅ Passed | The description is well-detailed and directly related to the changeset, explaining the UX improvement and technical implementation. |
✨ 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
feat/ADFA-2106-enable-log-text-selection
📜 Recent review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/src/main/java/com/itsaky/androidide/actions/file/ShowTooltipAction.kt(3 hunks)app/src/main/java/com/itsaky/androidide/fragments/EmptyStateFragment.kt(3 hunks)app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt(2 hunks)app/src/main/java/com/itsaky/androidide/fragments/output/LogViewFragment.kt(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/itsaky/androidide/fragments/EmptyStateFragment.kt (5)
app/src/main/java/com/itsaky/androidide/fragments/RecyclerViewFragment.kt (1)
onFragmentLongPressed(71-73)app/src/main/java/com/itsaky/androidide/agent/fragments/AgentFragmentContainer.kt (1)
onFragmentLongPressed(14-15)app/src/main/java/com/itsaky/androidide/agent/fragments/ChatFragment.kt (1)
onFragmentLongPressed(95-96)app/src/main/java/com/itsaky/androidide/fragments/debug/DebuggerFragment.kt (1)
onFragmentLongPressed(277-279)app/src/main/java/com/itsaky/androidide/fragments/sidebar/BuildVariantsFragment.kt (1)
onFragmentLongPressed(112-114)
🔇 Additional comments (6)
app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt (2)
36-45: Tagging build output editor with a concrete TooltipTag is good (avoids empty-tag pitfalls).
This plays nicely with the new “Help” action usingeditor.tagas the tooltip key.
23-27:currentEditoroverride type is correct. The parentNonEditableEditorFragment.getEditor()returnsIDEEditor?, and the binding's root view is explicitly typed asIDEEditorin the layout, so the override signaturecurrentEditor: IDEEditor?is properly aligned with the fragment hierarchy.app/src/main/java/com/itsaky/androidide/fragments/output/LogViewFragment.kt (1)
130-131:currentEditorwiring looks right for the new long-press selection behavior.app/src/main/java/com/itsaky/androidide/actions/file/ShowTooltipAction.kt (2)
42-52:prepare()gating visibility/enabled on selection is solid (and null-safe).
33-33: The review comment's concern about action ID rename verification is not applicable here. TheShowTooltipActionuses a stable ID"ide.editor.code.text.show_tooltip"with no evidence of any rename operation. The action is properly registered inEditorActivityActions.kt(line 110), and all string resources are correctly configured across localization files. No cleanup needed.app/src/main/java/com/itsaky/androidide/fragments/EmptyStateFragment.kt (1)
44-45: Nice direction:currentEditorhook + forwarding long-press coords is the right abstraction for selection-first UX.Also applies to: 67-69, 179-181
Comment @coderabbitai help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/src/main/java/com/itsaky/androidide/actions/file/ShowTooltipAction.kt(2 hunks)app/src/main/java/com/itsaky/androidide/fragments/EmptyStateFragment.kt(4 hunks)app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt(1 hunks)editor/src/main/java/com/itsaky/androidide/editor/ui/IDEEditor.kt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
editor/src/main/java/com/itsaky/androidide/editor/ui/IDEEditor.kt (1)
editor/src/main/java/com/itsaky/androidide/editor/ui/EditorFeatures.kt (2)
setSelection(40-44)setSelection(46-58)
app/src/main/java/com/itsaky/androidide/fragments/EmptyStateFragment.kt (6)
app/src/main/java/com/itsaky/androidide/fragments/RecyclerViewFragment.kt (1)
onFragmentLongPressed(71-73)app/src/main/java/com/itsaky/androidide/agent/fragments/AgentFragmentContainer.kt (1)
onFragmentLongPressed(14-15)app/src/main/java/com/itsaky/androidide/agent/fragments/ChatFragment.kt (1)
onFragmentLongPressed(95-96)app/src/main/java/com/itsaky/androidide/fragments/debug/DebuggerFragment.kt (1)
onFragmentLongPressed(277-279)app/src/main/java/com/itsaky/androidide/fragments/output/LogViewFragment.kt (1)
onFragmentLongPressed(284-286)app/src/main/java/com/itsaky/androidide/fragments/sidebar/BuildVariantsFragment.kt (1)
onFragmentLongPressed(112-114)
🔇 Additional comments (2)
app/src/main/java/com/itsaky/androidide/fragments/output/BuildOutputFragment.kt (1)
23-27: ExposecurrentEditorfor unified long-press selection flow (good).
This aligns the fragment with the newEmptyStateFragment.currentEditorextension point and enables coordinate-based selection without fragment-specific hacks.Please sanity-check on-device that after
onDestroyView()(Line 45-48), no long-press path can still reference a released editor instance (e.g., via delayed events).app/src/main/java/com/itsaky/androidide/actions/file/ShowTooltipAction.kt (1)
33-33: No action required. ShowTooltipAction is a newly added action, not a renamed one. The id "ide.editor.code.text.show_tooltip" has no prior references to update because this is a new action being introduced. The action is properly registered in EditorActivityActions.kt and all resource strings are in place.Likely an incorrect or invalid review comment.
app/src/main/java/com/itsaky/androidide/actions/file/ShowTooltipAction.kt
Show resolved
Hide resolved
…nd a double long-press
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/fragments/EmptyStateFragment.kt (1)
60-63: Consider renaming local variable to avoid shadowing.The local variable
currentEditorshadows the class property of the same name. While functionally correct, this can be confusing when reading the code.open fun onFragmentLongPressed() { - val currentEditor = currentEditor ?: return - currentEditor.selectCurrentWord() + val editor = currentEditor ?: return + editor.selectCurrentWord() }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/src/main/java/com/itsaky/androidide/actions/file/ShowTooltipAction.kt(2 hunks)app/src/main/java/com/itsaky/androidide/fragments/EmptyStateFragment.kt(4 hunks)editor/src/main/java/com/itsaky/androidide/editor/ui/IDEEditor.kt(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/main/java/com/itsaky/androidide/actions/file/ShowTooltipAction.kt
- editor/src/main/java/com/itsaky/androidide/editor/ui/IDEEditor.kt
🧰 Additional context used
🧬 Code graph analysis (1)
app/src/main/java/com/itsaky/androidide/fragments/EmptyStateFragment.kt (6)
app/src/main/java/com/itsaky/androidide/fragments/RecyclerViewFragment.kt (1)
onFragmentLongPressed(71-73)app/src/main/java/com/itsaky/androidide/agent/fragments/AgentFragmentContainer.kt (1)
onFragmentLongPressed(14-15)app/src/main/java/com/itsaky/androidide/agent/fragments/ChatFragment.kt (1)
onFragmentLongPressed(95-96)app/src/main/java/com/itsaky/androidide/fragments/debug/DebuggerFragment.kt (1)
onFragmentLongPressed(277-279)app/src/main/java/com/itsaky/androidide/fragments/output/LogViewFragment.kt (1)
onFragmentLongPressed(284-286)app/src/main/java/com/itsaky/androidide/fragments/sidebar/BuildVariantsFragment.kt (1)
onFragmentLongPressed(112-114)
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build Universal APK
🔇 Additional comments (3)
app/src/main/java/com/itsaky/androidide/fragments/EmptyStateFragment.kt (3)
45-46: LGTM!Clean extension point allowing subclasses to opt into coordinate-based selection behavior.
65-71: LGTM!The
isReadOnlyContextcheck correctly prevents double-dispatch of long-press handling in read-only editors, as discussed in the previous review.
178-182: LGTM!The event handler correctly extracts coordinates from the motion event for position-aware selection.
app/src/main/java/com/itsaky/androidide/fragments/EmptyStateFragment.kt
Outdated
Show resolved
Hide resolved
|
apply it in terminal too, previously couldn't highlight/select text properly when in terminal |
Thank you @hasanelfalakiy |
Description
This PR modifies the long-press behavior in the build output and log views to prioritize text selection over the immediate help tooltip.
Instead of blocking interaction with a "help" popup, a long-press now:
This resolves the UX conflict where developers were unable to copy text from logs.
Details
setSelectionFromPointto highlight the specific word or line.ShowTooltipActionso it appears in the floating menu when text is selected.BuildOutputFragmentnow exposes the underlying editor to handle coordinate-based selection.Screen.Recording.2025-12-12.at.3.44.18.PM.mov
Ticket
ADFA-2106
Observation
The
ShowTooltipActionwas updated to check foreditor.cursor.isSelected. The help icon will now only appear when there is an active selection in the editor/log view.