Open
Conversation
Contributor
dara-abijo-adfa
commented
Mar 11, 2026
dara-abijo-adfa
commented
- Show Retry button when Git clone fails
- Make error messages more descriptive
dara-abijo-adfa
requested review from
Daniel-ADFA,
itsaky-adfa,
jatezzz and
jomen-adfa
Contributor
📝 WalkthroughRelease Notes - ADFA-3074: Clone Error HandlingFeatures
Changes
Risks & Best Practice Considerations
WalkthroughThe pull request enhances repository cloning functionality with network error detection and recovery. The ViewModel implements connectivity checks and maps network exceptions to specific error states. The Fragment refactors clone logic into a reusable helper and adds retry button support. The layout undergoes substantial restructuring to support scrolling, local path selection, and authentication fields. String resources are migrated and expanded. Changes
Sequence DiagramsequenceDiagram
actor User
participant Fragment as CloneRepositoryFragment
participant ViewModel as CloneRepositoryViewModel
participant Network as NetworkUtils
participant Git as Git Operations
User->>Fragment: Click Clone Button
Fragment->>Fragment: cloneRepo() - gather inputs
Fragment->>ViewModel: cloneRepository(url, path, auth)
ViewModel->>Network: checkConnectivity()
alt No Internet
Network-->>ViewModel: No Connection
ViewModel-->>Fragment: Error(no_internet_connection)
Fragment->>Fragment: Show Retry Button
else Connected
ViewModel->>Git: Perform Clone
alt Success
Git-->>ViewModel: Clone Complete
ViewModel-->>Fragment: Success
else Network Exception
Git-->>ViewModel: TransportException/EOFException
ViewModel->>ViewModel: Map to Error Resource
ViewModel-->>Fragment: Error(connection_lost/unknown)
Fragment->>Fragment: Show Retry Button
end
end
User->>Fragment: Click Retry Button
Fragment->>Fragment: cloneRepo() - reuse helper
Fragment->>ViewModel: cloneRepository(same inputs)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/com/itsaky/androidide/fragments/CloneRepositoryFragment.kt (1)
165-182:⚠️ Potential issue | 🟡 MinorMissing
retryButtonvisibility handling in Success state.The
retryButtonvisibility is set in Idle, Cloning, and Error states but not in Success. While the Success state quickly transitions away, explicitly hiding it ensures consistent UI state.🛡️ Suggested fix
is CloneRepoUiState.Success -> { cloneButton.isEnabled = true + retryButton.visibility = View.GONE statusText.text = getString(R.string.clone_successful)🤖 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 165 - 182, In the CloneRepositoryFragment's handling of is CloneRepoUiState.Success, explicitly hide the retryButton (e.g., set its visibility to GONE or isVisible = false) before or when you reset the UI and call viewModel.resetState so the button state stays consistent with Idle/Cloning/Error branches; update the Success branch where cloneButton, statusText, destDir check, and viewModel.resetState() are handled to include hiding retryButton.
🧹 Nitpick comments (4)
resources/src/main/res/values/strings.xml (1)
1170-1177: Minor casing inconsistency indesc_file_conflicted.The string "Conflicted File" uses title case, while all other
desc_file_*strings use sentence case (e.g., "File added", "File modified"). Consider aligning the format for consistency.💅 Suggested fix
- <string name="desc_file_conflicted">Conflicted File</string> + <string name="desc_file_conflicted">File conflicted</string>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/src/main/res/values/strings.xml` around lines 1170 - 1177, The string resource desc_file_conflicted is inconsistent (uses "Conflicted File" title case) — update its value to match the other desc_file_* entries by using the same sentence/phrase form; specifically change the value for string name="desc_file_conflicted" to "File conflicted" so it aligns with "File added", "File modified", etc.app/src/main/java/com/itsaky/androidide/viewmodel/CloneRepositoryViewModel.kt (1)
163-165: String-based error detection is fragile but pragmatic.The message matching for
"Unexpected end of stream"and"Software caused connection abort"may differ across JGit versions and platforms. However, this is already preceded by anEOFExceptiontype check. If targeting more robustness, consider traversing the exception cause chain to check forSocketExceptionor otherIOExceptionsubtypes before falling back to message matching. Note that similar string-based matching patterns exist elsewhere in the codebase (e.g., WebServer.kt).🤖 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/viewmodel/CloneRepositoryViewModel.kt` around lines 163 - 165, Replace the fragile message-based detection in the isConnectionDrop logic by walking the exception cause chain (starting from the caught exception e) and checking for specific IO-related types like EOFException, SocketException or other IOException subclasses before falling back to string matching; update the check that currently uses e.cause and e.message to iterate through causes and return true on any matching exception type (refer to isConnectionDrop and the caught exception variable e) so the code is robust across JGit versions/platforms while retaining the existing message checks as a last resort.app/src/main/res/layout/fragment_clone_repository.xml (2)
112-146: Minor: Inconsistent ID naming convention.The button IDs mix naming styles:
exit_button(snake_case)retryButton(camelCase)cloneButton(camelCase)While View Binding handles
exit_buttoncorrectly (converts toexitButton), consistent naming improves maintainability. Consider standardizing on camelCase (exitButton) to match the others.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/res/layout/fragment_clone_repository.xml` around lines 112 - 146, Rename the inconsistent id exit_button to camelCase exitButton in the layout and update all usages in code to match (replace R.id.exit_button or any findViewById/reference and any viewBinding usages) so that ids for retryButton, cloneButton, and exitButton use the same naming convention; after changing the id, rebuild/run to fix any lingering references or generated binding names.
61-68: Consider using MaterialCheckBox for theme consistency.The layout uses Material3 styles throughout (
MaterialTextView,MaterialButton, MaterialTextInputLayout), but this CheckBox is the standard Android widget. Usingcom.google.android.material.checkbox.MaterialCheckBoxwould provide consistent theming and ripple behavior.♻️ Suggested change
- <CheckBox + <com.google.android.material.checkbox.MaterialCheckBox android:id="@+id/authCheckbox" android:layout_width="wrap_content" android:layout_height="wrap_content" android:layout_marginTop="16dp" android:text="@string/use_authentication" app:layout_constraintStart_toStartOf="parent" app:layout_constraintTop_toBottomOf="@id/localPathLayout" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/res/layout/fragment_clone_repository.xml` around lines 61 - 68, Replace the standard CheckBox in fragment_clone_repository.xml (id authCheckbox) with the Material component to match the rest of your Material3 widgets: change the XML tag to com.google.android.material.checkbox.MaterialCheckBox, keep the existing attributes (android:id="@+id/authCheckbox", android:layout_width, android:layout_height, android:layout_marginTop, android:text and constraint attributes) and ensure your app uses the Material components theme and the Material library dependency so the control inherits ripples and theming like MaterialTextView/MaterialButton.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@app/src/main/java/com/itsaky/androidide/fragments/CloneRepositoryFragment.kt`:
- Around line 165-182: In the CloneRepositoryFragment's handling of is
CloneRepoUiState.Success, explicitly hide the retryButton (e.g., set its
visibility to GONE or isVisible = false) before or when you reset the UI and
call viewModel.resetState so the button state stays consistent with
Idle/Cloning/Error branches; update the Success branch where cloneButton,
statusText, destDir check, and viewModel.resetState() are handled to include
hiding retryButton.
---
Nitpick comments:
In
`@app/src/main/java/com/itsaky/androidide/viewmodel/CloneRepositoryViewModel.kt`:
- Around line 163-165: Replace the fragile message-based detection in the
isConnectionDrop logic by walking the exception cause chain (starting from the
caught exception e) and checking for specific IO-related types like
EOFException, SocketException or other IOException subclasses before falling
back to string matching; update the check that currently uses e.cause and
e.message to iterate through causes and return true on any matching exception
type (refer to isConnectionDrop and the caught exception variable e) so the code
is robust across JGit versions/platforms while retaining the existing message
checks as a last resort.
In `@app/src/main/res/layout/fragment_clone_repository.xml`:
- Around line 112-146: Rename the inconsistent id exit_button to camelCase
exitButton in the layout and update all usages in code to match (replace
R.id.exit_button or any findViewById/reference and any viewBinding usages) so
that ids for retryButton, cloneButton, and exitButton use the same naming
convention; after changing the id, rebuild/run to fix any lingering references
or generated binding names.
- Around line 61-68: Replace the standard CheckBox in
fragment_clone_repository.xml (id authCheckbox) with the Material component to
match the rest of your Material3 widgets: change the XML tag to
com.google.android.material.checkbox.MaterialCheckBox, keep the existing
attributes (android:id="@+id/authCheckbox", android:layout_width,
android:layout_height, android:layout_marginTop, android:text and constraint
attributes) and ensure your app uses the Material components theme and the
Material library dependency so the control inherits ripples and theming like
MaterialTextView/MaterialButton.
In `@resources/src/main/res/values/strings.xml`:
- Around line 1170-1177: The string resource desc_file_conflicted is
inconsistent (uses "Conflicted File" title case) — update its value to match the
other desc_file_* entries by using the same sentence/phrase form; specifically
change the value for string name="desc_file_conflicted" to "File conflicted" so
it aligns with "File added", "File modified", etc.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: dcbb7850-fdea-4c82-8c29-611e9d07ba24
📒 Files selected for processing (5)
app/src/main/java/com/itsaky/androidide/fragments/CloneRepositoryFragment.ktapp/src/main/java/com/itsaky/androidide/viewmodel/CloneRepositoryViewModel.ktapp/src/main/res/layout/fragment_clone_repository.xmlapp/src/main/res/values/strings.xmlresources/src/main/res/values/strings.xml
💤 Files with no reviewable changes (1)
- app/src/main/res/values/strings.xml
jomen-adfa
approved these changes
Mar 12, 2026
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.