Skip to content

ADFA-3574 | Add drag and drop support for Git repository URLs#1132

Merged
jatezzz merged 2 commits intostagefrom
feat/ADFA-3125-git-url-drag-and-drop
Apr 2, 2026
Merged

ADFA-3574 | Add drag and drop support for Git repository URLs#1132
jatezzz merged 2 commits intostagefrom
feat/ADFA-3125-git-url-drag-and-drop

Conversation

@jatezzz
Copy link
Copy Markdown
Collaborator

@jatezzz jatezzz commented Mar 27, 2026

Description

Implemented a robust and reusable drag-and-drop infrastructure to allow users to drag Git repository URLs directly into the main screen or the clone fragment. This automatically initiates the cloning process, improving the onboarding experience for new projects.

Details

  • Core Architecture: Introduced DragEventRouter and DropTargetCallback to decouple complex Android DragEvent states from business logic.
  • Visual Feedback: Added DropHighlighter to provide a consistent visual cue (foreground overlay) when a valid drop target is active, preserving the original view state.
  • Git Integration:
    • Added GitUrlDropTarget and handleGitUrlDrop extensions to safely handle incoming URI and text drops.
    • Implemented a dedicated GitRepositoryUrls utility to validate and parse multiple Git schemes (HTTP, HTTPS, SSH, Git) while filtering out non-repository URLs.
  • Reactive Flow: Updated MainViewModel and CloneRepositoryViewModel to process dropped URLs via a Channel and Flow, ensuring a clean transition to the Clone fragment.

Quick Git Clone

document_5060103109758420762.mp4

Ticket

ADFA-3574
Parent: ADFA-3125

Observation

This PR serves as the foundational layer for Drag & Drop within the app. File-specific drag-and-drop logic for the File Tree (including NodeTouchHandler and FileImporter) has been extracted into a subsequent PR to facilitate a more focused code review.

@jatezzz jatezzz marked this pull request as draft March 27, 2026 23:03
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 27, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds drag-and-drop handling for Git repo URLs and external files, a Git URL parser/normalizer, ViewModel clone-request events, treeview drag initiation and touch handler, file-import utilities/controllers, and related UI strings and an ID.

Changes

Cohort / File(s) Summary
Git URL drop support
app/src/main/java/com/itsaky/androidide/dnd/GitUrlDropExtensions.kt, app/src/main/java/com/itsaky/androidide/dnd/GitUrlDropTarget.kt
New Fragment extension and internal drop target: lifecycle-aware attach/detach, drag highlight, accept text/URI payloads, parse/normalize Git URLs, invoke callback on normalized repo URL.
Drag event routing & helpers
app/src/main/java/com/itsaky/androidide/dnd/DragEventRouter.kt, app/src/main/java/com/itsaky/androidide/dnd/DropTargetCallback.kt, app/src/main/java/com/itsaky/androidide/dnd/DragAndDropExtensions.kt
Adds drag event router and DropTargetCallback interface plus helpers to detect importable external URIs from ClipData/DragEvent.
Git URL parsing (core)
git-core/src/main/java/com/itsaky/androidide/git/core/GitRepositoryUrls.kt
New public parseGitRepositoryUrl(rawText: String): String? — trims, accepts SSH shorthand, validates/normalizes http/https/git/ssh URIs, rejects web/UI links, returns normalized repo URL or null.
Clone/request flow
app/src/main/java/com/itsaky/androidide/viewmodel/MainViewModel.kt, app/src/main/java/com/itsaky/androidide/viewmodel/CloneRepositoryViewModel.kt, app/src/main/java/com/itsaky/androidide/fragments/CloneRepositoryFragment.kt, app/src/main/java/com/itsaky/androidide/fragments/MainFragment.kt
MainViewModel gains cloneRepositoryEvent and request method; clone VM validates/normalizes URLs and uses normalized URL in states; fragments observe clone events and wire drop handling to request clone.
File tree drag & drop (app)
app/src/main/java/com/itsaky/androidide/fragments/sidebar/FileTreeFragment.kt, app/src/main/java/com/itsaky/androidide/fragments/sidebar/FileTreeDropController.kt, app/src/main/java/com/itsaky/androidide/adapters/viewholders/FileTreeViewHolder.java, app/src/main/java/com/itsaky/androidide/tasks/callables/FileTreeCallable.java
File nodes can start drags via FileProvider URIs; new drop controller binds per-node/root targets, requests permissions, imports dropped URIs into project tree, refreshes nodes and reports success/failure; view holders now accept external drop handler.
TreeView touch/drag integration
treeview/src/main/java/com/unnamed/b/atv/model/TreeNode.java, treeview/src/main/java/com/unnamed/b/atv/view/AndroidTreeView.java, treeview/src/main/java/com/unnamed/b/atv/view/NodeTouchHandler.java
Adds TreeNodeDragListener and default-node drag listener API, refactors node click/long-click, and introduces NodeTouchHandler to detect gestures and dispatch drag starts.
URI import utilities & file importer
app/src/main/java/com/itsaky/androidide/utils/UriFileImporter.kt, app/src/main/java/com/itsaky/androidide/utils/FileImporter.kt, app/src/main/java/com/itsaky/androidide/viewmodels/PluginManagerViewModel.kt
New UriFileImporter for copying URIs and resolving display names; FileImporter to aggregate imports and report results; PluginManagerViewModel delegates URI copying and display-name resolution to UriFileImporter.
UI highlight & resources
app/src/main/java/com/itsaky/androidide/utils/DropHighlighter.kt, resources/src/main/res/values/ids.xml, resources/src/main/res/values/strings.xml
DropHighlighter manages temporary translucent teal highlight and restoration; adds resource id filetree_drop_target_tag and multiple strings for drag/drop outcomes and errors.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Fragment
    participant DropTarget
    participant Parser
    participant MainVM
    participant CloneFragment

    User->>Fragment: Drop text or URI onto view
    Fragment->>DropTarget: forward event (attached listener)
    DropTarget->>DropTarget: check shouldAcceptDrop()
    DropTarget->>Parser: parseGitRepositoryUrl(payload)
    alt valid normalized URL
        Parser-->>DropTarget: normalized URL
        DropTarget->>MainVM: requestCloneRepository(url)
        MainVM-->>MainVM: emit cloneRepositoryEvent, set screen
        MainVM->>CloneFragment: cloneRepositoryEvent observed
        CloneFragment->>CloneFragment: update repoUrl input & validate
    else not a repo URL
        Parser-->>DropTarget: null
        DropTarget-->>Fragment: clear highlight / ignore drop
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested reviewers

  • Daniel-ADFA
  • itsaky-adfa
  • jomen-adfa
  • dara-abijo-adfa

Poem

🐰
I nudged a url with a gentle hop,
Parsed and trimmed it—no need to stop.
Trees now dance when files come near,
Nibbles saved, imports cheer—🥕 hooray, my dear!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.48% 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 accurately summarizes the main change: adding drag-and-drop support for Git repository URLs, which is the primary focus of this changeset.
Description check ✅ Passed The PR description is directly related to the changeset, clearly explaining the drag-and-drop infrastructure implementation for Git URLs and file trees with architectural details and objectives.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/ADFA-3125-git-url-drag-and-drop

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

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (3)
app/src/main/java/com/itsaky/androidide/fragments/CloneRepositoryFragment.kt (1)

50-53: Factor the dropped/pending URL update into one helper.

These two blocks now duplicate the same repo-url propagation path. Keeping that logic in one place makes it less likely the drag/drop flow and the pending-event flow drift apart the next time this screen changes.

♻️ Suggested cleanup
-        handleGitUrlDrop { url ->
-            binding?.repoUrl?.setText(url)
-            viewModel.onInputChanged(url, binding?.localPath?.text?.toString().orEmpty())
-        }
+        handleGitUrlDrop(::applyRepositoryUrl)
@@
-                        binding?.repoUrl?.setText(trimmedUrl)
-                        viewModel.onInputChanged(trimmedUrl, binding?.localPath?.text?.toString().orEmpty())
+                        applyRepositoryUrl(trimmedUrl)
                     }
                 }
             }
         }
     }
+
+    private fun applyRepositoryUrl(url: String) {
+        binding?.repoUrl?.setText(url)
+        viewModel.onInputChanged(url, binding?.localPath?.text?.toString().orEmpty())
+    }

Also applies to: 210-218

🤖 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/CloneRepositoryFragment.kt`
around lines 50 - 53, Extract the duplicated repo-url propagation into a single
helper (e.g., updateRepoUrl(url: String)) and replace both the handleGitUrlDrop
callback and the pending-event block with calls to that helper; the helper
should set binding?.repoUrl?.setText(url) and call viewModel.onInputChanged(url,
binding?.localPath?.text?.toString().orEmpty()) so both flows (handleGitUrlDrop
and the other pending/url flow) share the same logic and cannot diverge.
app/src/main/java/com/itsaky/androidide/fragments/sidebar/FileTreeFragment.kt (1)

220-222: Prefer narrow exception handling.

Catching broad Exception masks potential issues. FileProvider.getUriForFile specifically throws IllegalArgumentException when the file is outside configured provider paths. Also, e.printStackTrace() is not ideal for production logging.

Based on learnings: prefer narrow exception handling that catches only the specific exception type (such as IllegalArgumentException) instead of a broad catch-all.

♻️ Suggested improvement
-    } catch (e: Exception) {
-      e.printStackTrace()
+    } catch (e: IllegalArgumentException) {
+      // File is outside FileProvider's configured paths
+      Log.w(TAG, "Cannot create content URI for file: ${file.path}", e)
     }
🤖 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/sidebar/FileTreeFragment.kt`
around lines 220 - 222, Replace the broad catch of Exception around the
FileProvider.getUriForFile call with a narrow catch for IllegalArgumentException
and remove e.printStackTrace(); instead log the error via the app logger (e.g.,
Log.e(TAG, "Failed to get Uri for file", e) or the existing logging facility) so
you only handle the specific provider-path error and record useful diagnostic
information; update the try/catch block in FileTreeFragment around the
FileProvider.getUriForFile usage accordingly.
treeview/src/main/java/com/unnamed/b/atv/view/AndroidTreeView.java (1)

440-451: Minor: Consider adding braces for consistency and readability.

The single-line if statements without braces work but reduce readability and can lead to maintenance issues.

♻️ Suggested formatting improvement
 private void handleNodeClick(TreeNode n) {
-    if (n.getClickListener() != null) n.getClickListener().onClick(n, n.getValue());
-    else if (nodeClickListener != null) nodeClickListener.onClick(n, n.getValue());
-    if (enableAutoToggle) toggleNode(n);
+    if (n.getClickListener() != null) {
+      n.getClickListener().onClick(n, n.getValue());
+    } else if (nodeClickListener != null) {
+      nodeClickListener.onClick(n, n.getValue());
+    }
+    if (enableAutoToggle) {
+      toggleNode(n);
+    }
 }

 private boolean handleNodeLongClick(TreeNode n) {
-    if (n.getLongClickListener() != null) return n.getLongClickListener().onLongClick(n, n.getValue());
-    if (nodeLongClickListener != null) return nodeLongClickListener.onLongClick(n, n.getValue());
-    if (enableAutoToggle) toggleNode(n);
+    if (n.getLongClickListener() != null) {
+      return n.getLongClickListener().onLongClick(n, n.getValue());
+    }
+    if (nodeLongClickListener != null) {
+      return nodeLongClickListener.onLongClick(n, n.getValue());
+    }
+    if (enableAutoToggle) {
+      toggleNode(n);
+    }
     return false;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@treeview/src/main/java/com/unnamed/b/atv/view/AndroidTreeView.java` around
lines 440 - 451, Update handleNodeClick and handleNodeLongClick to use braces
for all single-line if statements to improve consistency and readability: wrap
the bodies of the checks of n.getClickListener(), nodeClickListener,
n.getLongClickListener(), nodeLongClickListener, and the enableAutoToggle check
that calls toggleNode(n) in braces, preserving current logic and return values
(i.e., return the long-click listener result when present, otherwise proceed and
ultimately return false).
🤖 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/dnd/GitUrlDropTarget.kt`:
- Around line 46-50: The current flow uses extractDroppedText to take the first
non-blank clip item and then parseGitRepositoryUrl, which can fail if an earlier
clip entry is a label or content URI; update the logic in GitUrlDropTarget so
you iterate through all ClipData items (or all values returned by
extractDroppedText) and call parseGitRepositoryUrl on each until one returns a
non-null repositoryUrl, then call onRepositoryDropped(repositoryUrl) and return
null; ensure you preserve the existing early-returns only after exhausting all
items and apply the same change for the similar block referenced at lines
102-109.

In
`@app/src/main/java/com/itsaky/androidide/fragments/sidebar/FileTreeFragment.kt`:
- Around line 206-208: Replace the URL-based extension extraction with the file
object's extension property: instead of using
MimeTypeMap.getFileExtensionFromUrl(file.name) in FileTreeFragment (where
extension and mimeType are computed), use file.extension to get the extension
string and then pass that to
MimeTypeMap.getSingleton().getMimeTypeFromExtension(...) (fall back to
"application/octet-stream" as before); this ensures correct handling of
filenames with spaces or special characters.

In `@git-core/src/main/java/com/itsaky/androidide/git/core/GitRepositoryUrls.kt`:
- Around line 6-7: The SCP-style SSH regex special-cases the literal "git@"
(sshGitUrlRegex) so inputs like "alice@host:repo.git" are missed; update
sshGitUrlRegex to accept any non-space user identifier before the @ (e.g.,
replace the fixed "git@" with a generic "[^\\s@]+@") so SCP-form user@host:path
and ssh://user@host/... behave consistently; verify any other SCP checks in the
same file (the other occurrences you noted around lines 15-17) are changed to
use the same generalized pattern and keep supportedGitSchemes unchanged.
- Around line 35-37: The current heuristic (pathSegments.size >= 2 ||
path.endsWith(".git")) is too permissive and accepts URLs with extra path parts
or query strings (e.g., /org/repo/tree/..., ?tab=...), so update the check in
GitRepositoryUrls.kt (variables: path, pathSegments, looksLikeRepositoryPath) to
require no query and a canonical repo path: require uri.query.isNullOrBlank() &&
(pathSegments.size == 2 || path.endsWith(".git")), which ensures only bare
"org/repo" or explicit ".git" repo URLs enable cloning; adjust the same logic
used in the other block referenced (lines ~43-52) for consistency.

---

Nitpick comments:
In
`@app/src/main/java/com/itsaky/androidide/fragments/CloneRepositoryFragment.kt`:
- Around line 50-53: Extract the duplicated repo-url propagation into a single
helper (e.g., updateRepoUrl(url: String)) and replace both the handleGitUrlDrop
callback and the pending-event block with calls to that helper; the helper
should set binding?.repoUrl?.setText(url) and call viewModel.onInputChanged(url,
binding?.localPath?.text?.toString().orEmpty()) so both flows (handleGitUrlDrop
and the other pending/url flow) share the same logic and cannot diverge.

In
`@app/src/main/java/com/itsaky/androidide/fragments/sidebar/FileTreeFragment.kt`:
- Around line 220-222: Replace the broad catch of Exception around the
FileProvider.getUriForFile call with a narrow catch for IllegalArgumentException
and remove e.printStackTrace(); instead log the error via the app logger (e.g.,
Log.e(TAG, "Failed to get Uri for file", e) or the existing logging facility) so
you only handle the specific provider-path error and record useful diagnostic
information; update the try/catch block in FileTreeFragment around the
FileProvider.getUriForFile usage accordingly.

In `@treeview/src/main/java/com/unnamed/b/atv/view/AndroidTreeView.java`:
- Around line 440-451: Update handleNodeClick and handleNodeLongClick to use
braces for all single-line if statements to improve consistency and readability:
wrap the bodies of the checks of n.getClickListener(), nodeClickListener,
n.getLongClickListener(), nodeLongClickListener, and the enableAutoToggle check
that calls toggleNode(n) in braces, preserving current logic and return values
(i.e., return the long-click listener result when present, otherwise proceed and
ultimately return false).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6c35082d-2309-4ced-94b5-fa6768089b59

📥 Commits

Reviewing files that changed from the base of the PR and between 320665e and 07d8b6e.

📒 Files selected for processing (11)
  • app/src/main/java/com/itsaky/androidide/dnd/GitUrlDropExtensions.kt
  • app/src/main/java/com/itsaky/androidide/dnd/GitUrlDropTarget.kt
  • app/src/main/java/com/itsaky/androidide/fragments/CloneRepositoryFragment.kt
  • app/src/main/java/com/itsaky/androidide/fragments/MainFragment.kt
  • app/src/main/java/com/itsaky/androidide/fragments/sidebar/FileTreeFragment.kt
  • app/src/main/java/com/itsaky/androidide/viewmodel/CloneRepositoryViewModel.kt
  • app/src/main/java/com/itsaky/androidide/viewmodel/MainViewModel.kt
  • git-core/src/main/java/com/itsaky/androidide/git/core/GitRepositoryUrls.kt
  • treeview/src/main/java/com/unnamed/b/atv/model/TreeNode.java
  • treeview/src/main/java/com/unnamed/b/atv/view/AndroidTreeView.java
  • treeview/src/main/java/com/unnamed/b/atv/view/NodeTouchHandler.java

@jatezzz jatezzz marked this pull request as ready for review March 30, 2026 17:36
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 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/sidebar/FileTreeDropController.kt`:
- Around line 167-173: showDropHighlight replaces the view background and
clearDropHighlight nulls it, which removes row ripples; fix by saving the
original background in showDropHighlight (e.g. on the View via setTag with a
unique key) before applying the semi-transparent highlight, and in
clearDropHighlight restore that saved background (retrieving the tag) instead of
setting view.background = null; update showDropHighlight and clearDropHighlight
to use the same tag key and only set the saved background back when present.
- Around line 41-45: The drag handler is treating any non-empty drag as
importable via hasDroppedContent(), which incorrectly advertises self-drops
(URIs created by FileTreeFragment.onStartDrag) as valid; update handleDragEvent
(and the other similar checks around the file-tree drop logic) to inspect the
DragEvent/ClipData and exclude URIs that belong to our own FileProvider (or
other app-owned authorities) before returning true—i.e., compute canHandleDrop
by verifying there is at least one valid external/importable item and filtering
out any clip items whose URI authority equals our FileProvider/own-package
authority so self-drops are not highlighted and are not passed into
copyDroppedFiles().
- Around line 130-132: Sanitize the untrusted filename returned by
uri.getFileName(context) before passing it to createAvailableTargetFile/create
target File to prevent path traversal; compute sourceName by applying the
existing blank fallback, then strip or replace path separators ('/' and '\') and
any parent-segment patterns (".." or patterns like "../" or "/..") (e.g.,
replace separators with '_' and remove or neutralize leading/trailing ".."
segments) so that File(targetDirectory, sourceName) cannot escape the intended
directory; apply the same sanitization wherever the filename is used (the code
paths around createAvailableTargetFile and the File(directory, originalName)
usage noted in the diff).

In
`@app/src/main/java/com/itsaky/androidide/tasks/callables/FileTreeCallable.java`:
- Around line 52-58: populateChildren is called with the result of
File.listFiles(), which may return null and cause Arrays.sort to throw a
NullPointerException; add a null guard at the start of populateChildren(File[]
files, TreeNode parent) to return immediately (or skip sorting/processing) when
files is null or empty before calling Arrays.sort with SortFileName and
SortFolder, ensuring the method handles inaccessible/unreadable directories
gracefully.

In `@app/src/main/java/com/itsaky/androidide/utils/UriFileImporter.kt`:
- Around line 43-49: getDisplayName currently returns null when
queryDisplayName(contentResolver, uri) yields null for "content" URIs, which
causes downstream misnaming; update getDisplayName to fall back to
uri.lastPathSegment when queryDisplayName(...) is null. Specifically, in
getDisplayName (and the callsite logic relying on OpenableColumns.DISPLAY_NAME),
return queryDisplayName(contentResolver, uri) ?: uri.lastPathSegment for the
"content" branch so callers like PluginManagerViewModel.installPlugin receive a
sensible filename even if DISPLAY_NAME is absent.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8befafe8-b178-45da-b69d-439702b07a15

📥 Commits

Reviewing files that changed from the base of the PR and between 07d8b6e and 19b7dfc.

📒 Files selected for processing (9)
  • app/src/main/java/com/itsaky/androidide/adapters/viewholders/FileTreeViewHolder.java
  • app/src/main/java/com/itsaky/androidide/fragments/sidebar/FileTreeDropController.kt
  • app/src/main/java/com/itsaky/androidide/fragments/sidebar/FileTreeFragment.kt
  • app/src/main/java/com/itsaky/androidide/tasks/callables/FileTreeCallable.java
  • app/src/main/java/com/itsaky/androidide/utils/UriFileImporter.kt
  • app/src/main/java/com/itsaky/androidide/viewmodels/PluginManagerViewModel.kt
  • resources/src/main/res/values/ids.xml
  • resources/src/main/res/values/strings.xml
  • treeview/src/main/java/com/unnamed/b/atv/view/NodeTouchHandler.java
✅ Files skipped from review due to trivial changes (2)
  • resources/src/main/res/values/ids.xml
  • resources/src/main/res/values/strings.xml

@jatezzz jatezzz force-pushed the feat/ADFA-3125-git-url-drag-and-drop branch from 19b7dfc to d21fcec Compare March 30, 2026 20:05
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
treeview/src/main/java/com/unnamed/b/atv/view/AndroidTreeView.java (1)

148-150: Resolve the default drag listener at gesture time.

NodeTouchHandler stores whatever nodeDragListener is passed here, so setDefaultNodeDragListener(...) only affects nodes bound afterward. Using a lookup instead of a snapshot would make drag listener updates behave like click and long-click.

Also applies to: 426-426

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@treeview/src/main/java/com/unnamed/b/atv/view/AndroidTreeView.java` around
lines 148 - 150, setDefaultNodeDragListener currently stores a snapshot into
NodeTouchHandler so only nodes bound after the call see updates; change
NodeTouchHandler to resolve the listener at gesture time by looking up
AndroidTreeView.nodeDragListener (e.g., call a getter on AndroidTreeView or
reference the enclosing AndroidTreeView instance) instead of using the captured
listener reference created during binding. Update occurrences where
NodeTouchHandler captures the listener (including the similar case around the
other occurrence noted) so drag behavior reflects subsequent
setDefaultNodeDragListener calls.
🤖 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/sidebar/FileTreeFragment.kt`:
- Around line 242-244: Replace the broad catch in FileTreeFragment.kt with a
narrow one that only handles the expected drag-start error type (e.g., catch (e:
IllegalArgumentException)), remove the e.printStackTrace() call, and route the
error into the existing flashError(...) path (e.g., flashError(e.message ?:
"Failed to start drag")); do not swallow other exceptions—let them propagate by
not catching Exception broadly.

In `@treeview/src/main/java/com/unnamed/b/atv/view/NodeTouchHandler.java`:
- Around line 39-41: The drag is being started immediately on the first
ACTION_MOVE while isAwaitingDrag is true; instead, measure the movement delta
from the initial down coordinates and only call dispatchDrag() once the distance
exceeds the system touch slop. In NodeTouchHandler store the initial touch X/Y
on ACTION_DOWN, get the threshold via
ViewConfiguration.get(context).getScaledTouchSlop(), and in the ACTION_MOVE
branch (the block that checks isAwaitingDrag and calls dispatchDrag()) replace
the immediate dispatch with a check comparing the squared distance to the slop
(or a simple abs dx/dy check) and only clear isAwaitingDrag and call
dispatchDrag() when exceeded; apply the same change to the second similar block
referenced (the other isAwaitingDrag/ACTION_MOVE handling).

---

Nitpick comments:
In `@treeview/src/main/java/com/unnamed/b/atv/view/AndroidTreeView.java`:
- Around line 148-150: setDefaultNodeDragListener currently stores a snapshot
into NodeTouchHandler so only nodes bound after the call see updates; change
NodeTouchHandler to resolve the listener at gesture time by looking up
AndroidTreeView.nodeDragListener (e.g., call a getter on AndroidTreeView or
reference the enclosing AndroidTreeView instance) instead of using the captured
listener reference created during binding. Update occurrences where
NodeTouchHandler captures the listener (including the similar case around the
other occurrence noted) so drag behavior reflects subsequent
setDefaultNodeDragListener calls.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 416e203c-8364-405e-bb69-1df945211c92

📥 Commits

Reviewing files that changed from the base of the PR and between 19b7dfc and d21fcec.

📒 Files selected for processing (18)
  • app/src/main/java/com/itsaky/androidide/adapters/viewholders/FileTreeViewHolder.java
  • app/src/main/java/com/itsaky/androidide/dnd/GitUrlDropExtensions.kt
  • app/src/main/java/com/itsaky/androidide/dnd/GitUrlDropTarget.kt
  • app/src/main/java/com/itsaky/androidide/fragments/CloneRepositoryFragment.kt
  • app/src/main/java/com/itsaky/androidide/fragments/MainFragment.kt
  • app/src/main/java/com/itsaky/androidide/fragments/sidebar/FileTreeDropController.kt
  • app/src/main/java/com/itsaky/androidide/fragments/sidebar/FileTreeFragment.kt
  • app/src/main/java/com/itsaky/androidide/tasks/callables/FileTreeCallable.java
  • app/src/main/java/com/itsaky/androidide/utils/UriFileImporter.kt
  • app/src/main/java/com/itsaky/androidide/viewmodel/CloneRepositoryViewModel.kt
  • app/src/main/java/com/itsaky/androidide/viewmodel/MainViewModel.kt
  • app/src/main/java/com/itsaky/androidide/viewmodels/PluginManagerViewModel.kt
  • git-core/src/main/java/com/itsaky/androidide/git/core/GitRepositoryUrls.kt
  • resources/src/main/res/values/ids.xml
  • resources/src/main/res/values/strings.xml
  • treeview/src/main/java/com/unnamed/b/atv/model/TreeNode.java
  • treeview/src/main/java/com/unnamed/b/atv/view/AndroidTreeView.java
  • treeview/src/main/java/com/unnamed/b/atv/view/NodeTouchHandler.java
✅ Files skipped from review due to trivial changes (4)
  • resources/src/main/res/values/ids.xml
  • app/src/main/java/com/itsaky/androidide/fragments/MainFragment.kt
  • app/src/main/java/com/itsaky/androidide/utils/UriFileImporter.kt
  • resources/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (10)
  • app/src/main/java/com/itsaky/androidide/fragments/CloneRepositoryFragment.kt
  • app/src/main/java/com/itsaky/androidide/dnd/GitUrlDropExtensions.kt
  • app/src/main/java/com/itsaky/androidide/viewmodel/MainViewModel.kt
  • app/src/main/java/com/itsaky/androidide/viewmodels/PluginManagerViewModel.kt
  • app/src/main/java/com/itsaky/androidide/tasks/callables/FileTreeCallable.java
  • app/src/main/java/com/itsaky/androidide/viewmodel/CloneRepositoryViewModel.kt
  • app/src/main/java/com/itsaky/androidide/adapters/viewholders/FileTreeViewHolder.java
  • app/src/main/java/com/itsaky/androidide/dnd/GitUrlDropTarget.kt
  • git-core/src/main/java/com/itsaky/androidide/git/core/GitRepositoryUrls.kt
  • app/src/main/java/com/itsaky/androidide/fragments/sidebar/FileTreeDropController.kt

@jatezzz jatezzz force-pushed the feat/ADFA-3125-git-url-drag-and-drop branch from d21fcec to 8befaec Compare March 31, 2026 14:12
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
app/src/main/java/com/itsaky/androidide/fragments/sidebar/FileTreeDropController.kt (1)

151-159: ⚠️ Potential issue | 🟠 Major

Delete the half-written target before returning an import error.

The partial-success flow is better now, but the item that throws can still leave a truncated file behind. On retry, createAvailableTargetFile() will pick name (1) and the broken original stays in the tree.

💡 Minimal cleanup
       } catch (e: Exception) {
+        destinationFile.delete()
         return if (importedCount > 0) {
           ImportResult.PartialSuccess(importedCount, e)
         } else {
           ImportResult.Failure(e)
         }
       }
🤖 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/sidebar/FileTreeDropController.kt`
around lines 151 - 159, On import failure ensure any partially-written target is
removed: in the try/catch around UriFileImporter.copyUriToFile (the call that
uses destinationFile created by createAvailableTargetFile()), delete
destinationFile if it exists before rethrowing or propagating the error; this
guarantees a retry won't see a truncated file (i.e., in the catch for Exception
check destinationFile.exists() and delete it, then rethrow or handle the
original exception).
🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/fragments/CloneRepositoryFragment.kt (1)

50-53: Remove duplicate onInputChanged dispatch after setText.

Line 59 already updates ViewModel via repoUrl.doAfterTextChanged. Calling viewModel.onInputChanged(...) again in both handlers causes duplicate parsing/state updates for the same input.

♻️ Proposed cleanup
         handleGitUrlDrop { url ->
             binding?.repoUrl?.setText(url)
-            viewModel.onInputChanged(url, binding?.localPath?.text?.toString().orEmpty())
         }
@@
                 mainViewModel.cloneRepositoryEvent.collect { url ->
                     val trimmedUrl = url.trim()
                     if (trimmedUrl.isNotBlank()) {
                         binding?.repoUrl?.setText(trimmedUrl)
-                        viewModel.onInputChanged(trimmedUrl, binding?.localPath?.text?.toString().orEmpty())
                     }
                 }

Also applies to: 216-217

🤖 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/CloneRepositoryFragment.kt`
around lines 50 - 53, The drop handlers call viewModel.onInputChanged
immediately after setting the EditText text, causing duplicate updates because
repoUrl.doAfterTextChanged already dispatches onInputChanged; remove the
explicit viewModel.onInputChanged(...) calls from the handlers (e.g., in
handleGitUrlDrop where binding?.repoUrl?.setText(url) is followed by
viewModel.onInputChanged(...), and the analogous second handler mentioned at
lines 216-217) so that setText triggers the existing doAfterTextChanged flooring
the ViewModel update only once.
🤖 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/sidebar/FileTreeFragment.kt`:
- Around line 226-241: The call to ViewCompat.startDragAndDrop(...) in
FileTreeFragment currently ignores its boolean return value; update the
drag-start logic to capture the returned boolean (e.g., val started =
ViewCompat.startDragAndDrop(...)) and handle the failure case by showing user
feedback and/or logging (for example, show a Toast or call process logger) when
started is false; keep the existing ClipData, shadow, flags and error handling
around FileProvider.getUriForFile and MimeTypeMap, but ensure you react to a
failed startDrag in the same try block (or immediately after) so users are
informed if the drag could not be initiated.
- Around line 71-85: createFileNode() and code paths that run on a background
thread are reading fragment-scoped properties (Fragment.context /
requireActivity() via the externalDropHandler getter that uses dropController)
which can be null after onDestroyView(), so before dispatching any async listing
capture the needed references on the main thread (e.g., val context =
requireContext()/requireActivity(), val handler = externalDropHandler or
snapshot dropController.nodeBinder) and pass those snapshots into the worker
lambda; update all similar call sites including the createFileNode invocation
and the other spots noted (lines ~175–206) to use the captured values instead of
accessing externalDropHandler, dropController, or fragment context from the
background thread.

---

Duplicate comments:
In
`@app/src/main/java/com/itsaky/androidide/fragments/sidebar/FileTreeDropController.kt`:
- Around line 151-159: On import failure ensure any partially-written target is
removed: in the try/catch around UriFileImporter.copyUriToFile (the call that
uses destinationFile created by createAvailableTargetFile()), delete
destinationFile if it exists before rethrowing or propagating the error; this
guarantees a retry won't see a truncated file (i.e., in the catch for Exception
check destinationFile.exists() and delete it, then rethrow or handle the
original exception).

---

Nitpick comments:
In
`@app/src/main/java/com/itsaky/androidide/fragments/CloneRepositoryFragment.kt`:
- Around line 50-53: The drop handlers call viewModel.onInputChanged immediately
after setting the EditText text, causing duplicate updates because
repoUrl.doAfterTextChanged already dispatches onInputChanged; remove the
explicit viewModel.onInputChanged(...) calls from the handlers (e.g., in
handleGitUrlDrop where binding?.repoUrl?.setText(url) is followed by
viewModel.onInputChanged(...), and the analogous second handler mentioned at
lines 216-217) so that setText triggers the existing doAfterTextChanged flooring
the ViewModel update only once.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8baf483e-50b8-4a5a-b62e-446693cffdf3

📥 Commits

Reviewing files that changed from the base of the PR and between d21fcec and 8befaec.

📒 Files selected for processing (18)
  • app/src/main/java/com/itsaky/androidide/adapters/viewholders/FileTreeViewHolder.java
  • app/src/main/java/com/itsaky/androidide/dnd/GitUrlDropExtensions.kt
  • app/src/main/java/com/itsaky/androidide/dnd/GitUrlDropTarget.kt
  • app/src/main/java/com/itsaky/androidide/fragments/CloneRepositoryFragment.kt
  • app/src/main/java/com/itsaky/androidide/fragments/MainFragment.kt
  • app/src/main/java/com/itsaky/androidide/fragments/sidebar/FileTreeDropController.kt
  • app/src/main/java/com/itsaky/androidide/fragments/sidebar/FileTreeFragment.kt
  • app/src/main/java/com/itsaky/androidide/tasks/callables/FileTreeCallable.java
  • app/src/main/java/com/itsaky/androidide/utils/UriFileImporter.kt
  • app/src/main/java/com/itsaky/androidide/viewmodel/CloneRepositoryViewModel.kt
  • app/src/main/java/com/itsaky/androidide/viewmodel/MainViewModel.kt
  • app/src/main/java/com/itsaky/androidide/viewmodels/PluginManagerViewModel.kt
  • git-core/src/main/java/com/itsaky/androidide/git/core/GitRepositoryUrls.kt
  • resources/src/main/res/values/ids.xml
  • resources/src/main/res/values/strings.xml
  • treeview/src/main/java/com/unnamed/b/atv/model/TreeNode.java
  • treeview/src/main/java/com/unnamed/b/atv/view/AndroidTreeView.java
  • treeview/src/main/java/com/unnamed/b/atv/view/NodeTouchHandler.java
✅ Files skipped from review due to trivial changes (2)
  • resources/src/main/res/values/ids.xml
  • app/src/main/java/com/itsaky/androidide/fragments/MainFragment.kt
🚧 Files skipped from review as they are similar to previous changes (9)
  • app/src/main/java/com/itsaky/androidide/viewmodel/MainViewModel.kt
  • app/src/main/java/com/itsaky/androidide/viewmodel/CloneRepositoryViewModel.kt
  • treeview/src/main/java/com/unnamed/b/atv/model/TreeNode.java
  • resources/src/main/res/values/strings.xml
  • app/src/main/java/com/itsaky/androidide/dnd/GitUrlDropExtensions.kt
  • app/src/main/java/com/itsaky/androidide/viewmodels/PluginManagerViewModel.kt
  • git-core/src/main/java/com/itsaky/androidide/git/core/GitRepositoryUrls.kt
  • app/src/main/java/com/itsaky/androidide/dnd/GitUrlDropTarget.kt
  • treeview/src/main/java/com/unnamed/b/atv/view/NodeTouchHandler.java

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (6)
app/src/main/java/com/itsaky/androidide/utils/DropHighlighter.kt (1)

13-19: Consider extracting the alpha value and sentinel string as named constants.

The magic number 64 for alpha opacity and the sentinel string "NULL_BG" could be extracted to named constants for improved readability and to prevent potential typos if this sentinel value is ever referenced elsewhere.

♻️ Suggested refactor
 object DropHighlighter {
+    private const val NULL_BG_SENTINEL = "NULL_BG"
+    private const val HIGHLIGHT_ALPHA = 64 // ~25% opacity (64/255)
+
     /**
      * Applies a highlight background to the [view] to indicate an active drop target, saving its original background.
      */
     fun highlight(view: View, context: Context) {
         if (view.getTag(R.id.filetree_drop_target_tag) == null) {
-            view.setTag(R.id.filetree_drop_target_tag, view.background ?: "NULL_BG")
+            view.setTag(R.id.filetree_drop_target_tag, view.background ?: NULL_BG_SENTINEL)
         }
         val baseColor = ContextCompat.getColor(context, R.color.teal_200)
-        view.setBackgroundColor((baseColor and 0x00FFFFFF) or (64 shl 24))
+        view.setBackgroundColor((baseColor and 0x00FFFFFF) or (HIGHLIGHT_ALPHA shl 24))
     }
🤖 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/utils/DropHighlighter.kt` around
lines 13 - 19, Extract the magic alpha value and the sentinel string into named
constants to improve readability and avoid typos: define a private constant
(e.g., DROP_HIGHLIGHT_ALPHA = 64) and another for the sentinel (e.g.,
NULL_BACKGROUND_SENTINEL = "NULL_BG") and replace the literal 64 and "NULL_BG"
usages in the highlight(view: View, context: Context) function where you set the
tag for R.id.filetree_drop_target_tag and compute the background color; keep all
other behavior unchanged.
app/src/main/java/com/itsaky/androidide/dnd/DragEventRouter.kt (2)

33-45: onDragExited called twice per successful drop or exit sequence.

In the current implementation:

  • Drop flow: ACTION_DROP (line 34) → ACTION_DRAG_ENDED (line 43) = 2 calls
  • Exit flow: ACTION_DRAG_EXITED (line 29) → ACTION_DRAG_ENDED (line 43) = 2 calls

This ensures cleanup always occurs (defensive), but requires callbacks to be idempotent. Looking at FileTreeDropController (context snippet 2), DropHighlighter.clear(view) appears safe to call multiple times.

If you'd prefer to avoid redundant calls, consider tracking whether exit was already signaled. Otherwise, document that onDragExited may be invoked multiple times per drag session.

🤖 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/dnd/DragEventRouter.kt` around lines
33 - 45, The code calls callback.onDragExited twice for a single drag sequence
(once in the ACTION_DROP branch and again in ACTION_DRAG_ENDED), so add simple
state to prevent duplicate exit notifications: introduce a boolean flag (e.g.,
exitSignaled) on DragEventRouter that is cleared at the start of a drag and set
when you call callback.onDragExited, then only call onDragExited in the
ACTION_DROP, ACTION_DRAG_EXITED and ACTION_DRAG_ENDED handlers if exitSignaled
is false; ensure the flag is reset when a new drag begins so subsequent drags
behave normally.

19-31: Asymmetric callback invocation between onDragEntered and onDragExited.

onDragEntered is only called when canHandle is true (line 20), but onDragExited is called unconditionally (line 29). If canAcceptDrop returns different values across events (e.g., true for ACTION_DRAG_STARTED but false for ACTION_DRAG_ENTERED), this could result in onDragExited being called without a corresponding onDragEntered.

Consider guarding onDragExited similarly, or documenting that callbacks must be idempotent and tolerate unpaired exit calls.

♻️ Optional: Make exit calls symmetric with enter
             DragEvent.ACTION_DRAG_EXITED -> {
-                callback.onDragExited(view)
+                if (canHandle) callback.onDragExited(view)
                 true
             }
🤖 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/dnd/DragEventRouter.kt` around lines
19 - 31, The onDragExited callback is invoked unconditionally while
onDragEntered is only called when canHandle is true, which can produce unpaired
exit events; update the DragEventRouter handler so ACTION_DRAG_EXITED also
checks the same condition (canHandle / result of canAcceptDrop) before calling
callback.onDragExited(view) to make enter/exit symmetric, or alternatively add a
comment in the DragEventRouter documenting that callback implementations must be
idempotent and tolerate unpaired exits (referencing callback.onDragEntered,
callback.onDragExited, and the canHandle / canAcceptDrop check).
app/src/main/java/com/itsaky/androidide/fragments/sidebar/FileTreeFragment.kt (2)

297-300: Use consistent context access for both tree nodes.

Line 297 uses requireContext() while line 300 uses nullable context. For consistency and safety, use requireContext() on both lines or capture the context once and reuse it.

♻️ Suggested fix
+    val ctx = requireContext()
     val rootNode = TreeNode(File(""))
-    rootNode.viewHolder = FileTreeViewHolder(requireContext(), externalDropHandler)
+    rootNode.viewHolder = FileTreeViewHolder(ctx, externalDropHandler)

     val projectRoot = TreeNode.root(projectDir)
-    projectRoot.viewHolder = FileTreeViewHolder(context, externalDropHandler)
+    projectRoot.viewHolder = FileTreeViewHolder(ctx, externalDropHandler)
🤖 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/sidebar/FileTreeFragment.kt`
around lines 297 - 300, The two TreeNode viewHolder assignments use inconsistent
context access (rootNode.viewHolder uses requireContext() while
projectRoot.viewHolder uses nullable context); fix by consistently using
requireContext() or by capturing a non-null Context once (e.g., val ctx =
requireContext()) and then pass ctx to both FileTreeViewHolder calls (affecting
rootNode.viewHolder and projectRoot.viewHolder where FileTreeViewHolder is
constructed with projectDir/externalDropHandler).

249-252: Narrow the exception catch to specific expected types.

The broad catch (Exception) hides unrelated bugs and conflicts with the project's fail-fast preference. FileProvider.getUriForFile() typically throws IllegalArgumentException. Catch only the expected exception type(s) and let others propagate.

♻️ Suggested fix
-    } catch (e: Exception) {
-      e.printStackTrace()
+    } catch (e: IllegalArgumentException) {
       flashError(getString(R.string.msg_file_tree_drag_error))
     }

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
`@app/src/main/java/com/itsaky/androidide/fragments/sidebar/FileTreeFragment.kt`
around lines 249 - 252, The broad catch of Exception in FileTreeFragment (around
the FileProvider.getUriForFile() call) should be narrowed to catch only the
expected exception type(s) (e.g., IllegalArgumentException) so unrelated errors
can propagate; replace "catch (e: Exception) { e.printStackTrace();
flashError(getString(R.string.msg_file_tree_drag_error)) }" with a specific
"catch (e: IllegalArgumentException) { /* optional logging */
flashError(getString(R.string.msg_file_tree_drag_error)) }" (remove the
printStackTrace) and do not swallow other exceptions so they can surface/fail
fast.
app/src/main/java/com/itsaky/androidide/dnd/DragAndDropExtensions.kt (1)

36-41: Handle potential IllegalArgumentException from toUri() on malformed text.

The toUri() extension can throw if the text starts with content:// or file:// but contains malformed URI syntax. Consider wrapping the parse in runCatching to gracefully return null on invalid input.

♻️ Suggested fix
 private fun ClipData.Item.toExternalUri(): Uri? {
     return uri
         ?: text?.toString()
             ?.takeIf { it.startsWith("content://") || it.startsWith("file://") }
-            ?.toUri()
+            ?.let { runCatching { it.toUri() }.getOrNull() }
 }
🤖 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/dnd/DragAndDropExtensions.kt` around
lines 36 - 41, In ClipData.Item.toExternalUri(), avoid crashing on malformed
text URIs by wrapping the text-to-Uri conversion in a safe parser (e.g., use
runCatching or try/catch around text.toString().toUri()) and return null on
failure; update the logic inside the toExternalUri() function so it still
prefers the existing uri, but when falling back to text that startsWith
"content://" or "file://", parse it defensively and return null if parsing
throws an IllegalArgumentException.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@app/src/main/java/com/itsaky/androidide/dnd/DragAndDropExtensions.kt`:
- Around line 36-41: In ClipData.Item.toExternalUri(), avoid crashing on
malformed text URIs by wrapping the text-to-Uri conversion in a safe parser
(e.g., use runCatching or try/catch around text.toString().toUri()) and return
null on failure; update the logic inside the toExternalUri() function so it
still prefers the existing uri, but when falling back to text that startsWith
"content://" or "file://", parse it defensively and return null if parsing
throws an IllegalArgumentException.

In `@app/src/main/java/com/itsaky/androidide/dnd/DragEventRouter.kt`:
- Around line 33-45: The code calls callback.onDragExited twice for a single
drag sequence (once in the ACTION_DROP branch and again in ACTION_DRAG_ENDED),
so add simple state to prevent duplicate exit notifications: introduce a boolean
flag (e.g., exitSignaled) on DragEventRouter that is cleared at the start of a
drag and set when you call callback.onDragExited, then only call onDragExited in
the ACTION_DROP, ACTION_DRAG_EXITED and ACTION_DRAG_ENDED handlers if
exitSignaled is false; ensure the flag is reset when a new drag begins so
subsequent drags behave normally.
- Around line 19-31: The onDragExited callback is invoked unconditionally while
onDragEntered is only called when canHandle is true, which can produce unpaired
exit events; update the DragEventRouter handler so ACTION_DRAG_EXITED also
checks the same condition (canHandle / result of canAcceptDrop) before calling
callback.onDragExited(view) to make enter/exit symmetric, or alternatively add a
comment in the DragEventRouter documenting that callback implementations must be
idempotent and tolerate unpaired exits (referencing callback.onDragEntered,
callback.onDragExited, and the canHandle / canAcceptDrop check).

In
`@app/src/main/java/com/itsaky/androidide/fragments/sidebar/FileTreeFragment.kt`:
- Around line 297-300: The two TreeNode viewHolder assignments use inconsistent
context access (rootNode.viewHolder uses requireContext() while
projectRoot.viewHolder uses nullable context); fix by consistently using
requireContext() or by capturing a non-null Context once (e.g., val ctx =
requireContext()) and then pass ctx to both FileTreeViewHolder calls (affecting
rootNode.viewHolder and projectRoot.viewHolder where FileTreeViewHolder is
constructed with projectDir/externalDropHandler).
- Around line 249-252: The broad catch of Exception in FileTreeFragment (around
the FileProvider.getUriForFile() call) should be narrowed to catch only the
expected exception type(s) (e.g., IllegalArgumentException) so unrelated errors
can propagate; replace "catch (e: Exception) { e.printStackTrace();
flashError(getString(R.string.msg_file_tree_drag_error)) }" with a specific
"catch (e: IllegalArgumentException) { /* optional logging */
flashError(getString(R.string.msg_file_tree_drag_error)) }" (remove the
printStackTrace) and do not swallow other exceptions so they can surface/fail
fast.

In `@app/src/main/java/com/itsaky/androidide/utils/DropHighlighter.kt`:
- Around line 13-19: Extract the magic alpha value and the sentinel string into
named constants to improve readability and avoid typos: define a private
constant (e.g., DROP_HIGHLIGHT_ALPHA = 64) and another for the sentinel (e.g.,
NULL_BACKGROUND_SENTINEL = "NULL_BG") and replace the literal 64 and "NULL_BG"
usages in the highlight(view: View, context: Context) function where you set the
tag for R.id.filetree_drop_target_tag and compute the background color; keep all
other behavior unchanged.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 86febc2e-474b-4251-9abd-99297c6fae53

📥 Commits

Reviewing files that changed from the base of the PR and between 8befaec and f326b8a.

📒 Files selected for processing (8)
  • app/src/main/java/com/itsaky/androidide/dnd/DragAndDropExtensions.kt
  • app/src/main/java/com/itsaky/androidide/dnd/DragEventRouter.kt
  • app/src/main/java/com/itsaky/androidide/dnd/DropTargetCallback.kt
  • app/src/main/java/com/itsaky/androidide/fragments/sidebar/FileTreeDropController.kt
  • app/src/main/java/com/itsaky/androidide/fragments/sidebar/FileTreeFragment.kt
  • app/src/main/java/com/itsaky/androidide/utils/DropHighlighter.kt
  • app/src/main/java/com/itsaky/androidide/utils/FileImporter.kt
  • resources/src/main/res/values/strings.xml
✅ Files skipped from review due to trivial changes (2)
  • app/src/main/java/com/itsaky/androidide/dnd/DropTargetCallback.kt
  • app/src/main/java/com/itsaky/androidide/fragments/sidebar/FileTreeDropController.kt
🚧 Files skipped from review as they are similar to previous changes (1)
  • resources/src/main/res/values/strings.xml

@jatezzz jatezzz force-pushed the feat/ADFA-3125-git-url-drag-and-drop branch 2 times, most recently from 5f7da2d to 5fb4867 Compare April 1, 2026 17:48
@jatezzz jatezzz requested a review from Daniel-ADFA April 1, 2026 17:49
@jatezzz jatezzz changed the title ADFA-3125 | Add drag and drop support for Git repository URLs ADFA-3574 | Add drag and drop support for Git repository URLs Apr 1, 2026
@jatezzz jatezzz requested a review from itsaky-adfa April 2, 2026 13:27
@jatezzz jatezzz force-pushed the feat/ADFA-3125-git-url-drag-and-drop branch from 449d472 to 8cb0114 Compare April 2, 2026 21:06
@jatezzz jatezzz merged commit cb05a7d into stage Apr 2, 2026
2 checks passed
@jatezzz jatezzz deleted the feat/ADFA-3125-git-url-drag-and-drop branch April 2, 2026 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants