ADFA-3133 Bug fix for over-eager auto-save during onPause event#1151
Merged
hal-eisen-adfa merged 3 commits intostagefrom Apr 8, 2026
Merged
Conversation
Contributor
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughRelease Notes - Bug Fix for Over-Eager Auto-Save During onPause EventKey Improvements
Modified Files
Risks & Best-Practice Notes
WalkthroughEditor configuration handling adjusted to avoid activity recreation for font scale changes; editor lifecycle/save logic and external-change reloads tightened; editor display preferences are reapplied after configuration changes; RecyclerView fragment binding replaced with a manual ViewBinding and tab indexing fixes were applied. Changes
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 docstrings
🧪 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.
Actionable comments posted: 2
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/activities/editor/EditorHandlerActivity.kt (1)
235-248:⚠️ Potential issue | 🟠 MajorSnapshot mtimes before leaving
onPause().Because this snapshot is launched asynchronously, a write that happens just after
onPause()can win the race and become the stored baseline. When that happens,checkForExternalFileChanges()sees no delta on resume and misses the external change entirely.Suggested fix
override fun onPause() { super.onPause() - // Record timestamps for all currently open files before saving the cache - val openFiles = editorViewModel.getOpenedFiles() - lifecycleScope.launch(Dispatchers.IO) { - openFiles.forEach { file -> - // Note: Using the file's absolutePath as the key - fileTimestamps[file.absolutePath] = file.lastModified() - } - } + editorViewModel.getOpenedFiles().forEach { file -> + fileTimestamps[file.absolutePath] = file.lastModified() + } ActionContextProvider.clearActivity() if (!isOpenedFilesSaved.get()) { saveOpenedFiles()🤖 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/activities/editor/EditorHandlerActivity.kt` around lines 235 - 248, The current onPause() launches an async coroutine to record fileTimestamps which allows a concurrent write to race and become the baseline; instead capture a synchronous snapshot of opened file paths and their lastModified() values on the caller thread before launching any background work, then hand that immutable snapshot into lifecycleScope.launch(Dispatchers.IO) to persist it; update the onPause() flow that calls editorViewModel.getOpenedFiles(), writes into fileTimestamps, and uses lifecycleScope.launch(Dispatchers.IO) so the snapshot is computed synchronously but the disk/storage persistence remains asynchronous.
🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/fragments/RecyclerViewFragment.kt (1)
168-175:attachToParentparameter is ignored.The
inflate()method acceptsattachToParentbut always passesfalsetoinflater.inflate(). While current callers inFragmentWithBindingalways passfalse, this deviates from standardViewBinding.inflate()behavior and could be misleading.Consider either honoring the parameter (with appropriate handling for the
truecase whereinflate()returns the parent) or adding a comment explaining why it's intentionally ignored.💡 Option: Add clarifying comment
fun inflate( inflater: LayoutInflater, parent: ViewGroup?, attachToParent: Boolean, ): FragmentRecyclerviewManualBinding { + // Note: attachToParent is intentionally ignored; FragmentWithBinding always passes false. + // Honoring true would require special handling since inflate() returns parent, not the inflated view. val root = inflater.inflate(R.layout.fragment_recyclerview, parent, false) as RecyclerView return FragmentRecyclerviewManualBinding(root) }🤖 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/RecyclerViewFragment.kt` around lines 168 - 175, The inflate() function in FragmentRecyclerviewManualBinding currently ignores the attachToParent parameter and always calls inflater.inflate(..., false); update inflate(inflater: LayoutInflater, parent: ViewGroup?, attachToParent: Boolean) to either honor attachToParent (i.e., pass attachToParent into inflater.inflate and handle the returned View when attachToParent is true) or add a clear comment above FragmentRecyclerviewManualBinding.inflate explaining why attachToParent is intentionally ignored; locate the inflater.inflate(...) call in the inflate method and either replace the hardcoded false with the attachToParent parameter and adjust return handling for the parent-attached case, or add the explanatory comment referencing FragmentRecyclerviewManualBinding.inflate to justify the current behavior.
🤖 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/AndroidManifest.xml`:
- Around line 95-97: The manifest opts out of uiMode and density configuration
changes for the activity declared as
android:name=".activities.editor.EditorActivityKt", but onConfigurationChanged()
only calls configureEditorIfNeeded()/reapplyEditorDisplayPreferences() and does
not rebind theme/density-dependent state (e.g.,
SchemeAndroidIDE.newInstance(context), CodeEditorView initialization or
SizeUtils.dp2px-based drawables), so remove uiMode and density from the
android:configChanges attribute (leave fontScale so system font-scale handling
remains) to force activity recreation until full UI rebinding is implemented.
In
`@app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt`:
- Around line 277-302: The reload is incorrectly gated by
IDEEditor.canUndo()/canRedo() so clean buffers that still have undo history
(after markAsSaved()/markUnmodified()) are skipped; in
checkForExternalFileChanges() remove or replace the early return that checks
ideEditor.canUndo() || ideEditor.canRedo() so that when editorView.isModified is
false the file is reloaded (call ideEditor.setText(newContent) and
editorView.markAsSaved()); if you must preserve undo history instead of
discarding it, replace that early-return with an explicit user conflict
prompt/confirmation flow (show dialog) rather than silently ignoring the
external change—refer to checkForExternalFileChanges(),
CodeEditorView.markAsSaved(), and IDEEditor.canUndo()/canRedo()/markUnmodified()
when making the change.
---
Outside diff comments:
In
`@app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt`:
- Around line 235-248: The current onPause() launches an async coroutine to
record fileTimestamps which allows a concurrent write to race and become the
baseline; instead capture a synchronous snapshot of opened file paths and their
lastModified() values on the caller thread before launching any background work,
then hand that immutable snapshot into lifecycleScope.launch(Dispatchers.IO) to
persist it; update the onPause() flow that calls
editorViewModel.getOpenedFiles(), writes into fileTimestamps, and uses
lifecycleScope.launch(Dispatchers.IO) so the snapshot is computed synchronously
but the disk/storage persistence remains asynchronous.
---
Nitpick comments:
In `@app/src/main/java/com/itsaky/androidide/fragments/RecyclerViewFragment.kt`:
- Around line 168-175: The inflate() function in
FragmentRecyclerviewManualBinding currently ignores the attachToParent parameter
and always calls inflater.inflate(..., false); update inflate(inflater:
LayoutInflater, parent: ViewGroup?, attachToParent: Boolean) to either honor
attachToParent (i.e., pass attachToParent into inflater.inflate and handle the
returned View when attachToParent is true) or add a clear comment above
FragmentRecyclerviewManualBinding.inflate explaining why attachToParent is
intentionally ignored; locate the inflater.inflate(...) call in the inflate
method and either replace the hardcoded false with the attachToParent parameter
and adjust return handling for the parent-attached case, or add the explanatory
comment referencing FragmentRecyclerviewManualBinding.inflate to justify the
current behavior.
🪄 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: 7a62c8af-bb8d-4ba8-84d9-8ba366b6abb2
📒 Files selected for processing (4)
app/src/main/AndroidManifest.xmlapp/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.ktapp/src/main/java/com/itsaky/androidide/fragments/RecyclerViewFragment.ktapp/src/main/java/com/itsaky/androidide/ui/CodeEditorView.kt
…Mode/density unless resources are fully rebound
jomen-adfa
approved these changes
Apr 8, 2026
hal-eisen-adfa
deleted the
ADFA-3133-Editor-writes-files-without-user-assent
branch
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.
Remove implicit save on pause — Delete or gate saveAllAsync in onPause; do not write project files there. Re-evaluate writeOpenedFilesCache / PREF_KEY_OPEN_FILES_CACHE: session restore should not reopen or replace editors in a way that drops undo; prefer remembering paths + selection only when cold-starting, not clobbering live buffers on every onStart after pause.
Resume must not clobber editors — Rework or remove automatic “disk is newer → setText” for open dirty workflows; if external change detection remains, use an explicit conflict dialog, never silent replace while the user may have unsaved work.
Fix updateTabs() — For each non-plugin tab position, use getFileIndexForTabPosition then getEditorAtIndex(fileIndex) for isModified and naming.
Configuration / font scale — EditorActivityKt does not list fontScale; system font changes recreate the activity and lose undo. Prefer adding fontScale (and applying font from preferences in onConfigurationChanged / CodeEditorView) so editors are not torn down, or document that full parity requires a larger state-save story.
Audit — Search for other saveAll, writeTo, or sync triggers tied to lifecycle that are not explicit save or prompted flows.