ADFA-3133 Bug fix for over-eager auto-save during onPause event#1151
ADFA-3133 Bug fix for over-eager auto-save during onPause event#1151hal-eisen-adfa wants to merge 1 commit intostagefrom
Conversation
📝 WalkthroughRelease Notes - Bug Fix for Over-Eager Auto-Save During onPause EventKey Improvements
Modified Files
|
| Cohort / File(s) | Summary |
|---|---|
Configuration & Display Preferences app/src/main/AndroidManifest.xml, app/src/main/java/com/itsaky/androidide/ui/CodeEditorView.kt |
Extended android:configChanges on EditorActivity to include fontScale, density, and uiMode. Added reapplyEditorDisplayPreferences() method to CodeEditorView to re-invoke preference handlers after configuration changes. |
Editor Lifecycle & State Management app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt |
Modified onPause() to persist only open-tab state without implicit file saves. Enhanced checkForExternalFileChanges() with stricter gating (disk timestamp + editor unmodified checks) to prevent overwrites during undo/redo. Added startup logic to skip reopening persisted files when editor already has open tabs. Updated onConfigurationChanged() to reapply editor display preferences. Refined updateTabs() to account for plugin tabs with corrected tab-to-file indexing. |
View Binding app/src/main/java/com/itsaky/androidide/fragments/RecyclerViewFragment.kt |
Introduced custom FragmentRecyclerviewManualBinding to replace generated binding, with RecyclerView exposed as binding root. Updated RecyclerView access from binding.root to binding.list throughout the fragment. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
- PR
#1057: Modifies EditorHandlerActivity's startup and opened-files cache logic with validation scoping - PR
#993: Updates EditorActivity's configuration-change handling and orientation behavior - PR
#735: Modifies EditorHandlerActivity's onPause and file-save behavior during lifecycle events
Suggested reviewers
- itsaky-adfa
- jatezzz
- Daniel-ADFA
Poem
🐰 Hopping through configurations with grace,
Preferences dance in their rightful place,
Files persist, not forced to the ground,
Tab indices bloom without making a sound,
The editor springs back, refreshed and bright! ✨
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 31.25% 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 title accurately summarizes the primary change: fixing over-eager auto-save during onPause, which is the core issue addressed across the codebase modifications. |
| Description check | ✅ Passed | The description is directly related to the changeset, providing detailed context about the auto-save behavior, session restoration, external file change handling, tab updates, configuration changes, and font scaling issues that are all reflected in the code modifications. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
ADFA-3133-Editor-writes-files-without-user-assent
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 help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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
| android:name=".activities.editor.EditorActivityKt" | ||
| android:configChanges="orientation|screenSize|screenLayout|smallestScreenSize" | ||
| android:configChanges="orientation|screenSize|screenLayout|smallestScreenSize|fontScale|density|uiMode" | ||
| android:launchMode="singleTask" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
When an Android activity handles uiModeanddensityviaandroid:configChanges, what theme/resource updates must it perform manually in onConfigurationChanged to avoid stale dark-mode and density-dependent UI?
💡 Result:
When an Android activity declares android:configChanges="uiMode|density" in its manifest, it must manually update its UI in onConfigurationChanged(Configuration newConfig) to reflect changes in dark mode (uiMode) and screen density, as the system does not recreate the activity. Key manual updates required: 1. Reload layouts and resources: Call setContentView(R.layout.activity_main) to reload the layout from the appropriate resource folder (e.g., layout-night for dark mode, density-specific drawables). This ensures density-dependent bitmaps and dimension resources are updated. 2. Re-apply theme attributes: For views using theme colors/text colors (e.g., ?attr/colorPrimary), reload backgrounds, text colors, etc., via view.setBackgroundResource or ContextCompat.getColor after setContentView, as theme application may not fully update. 3. Update custom views: Recreate or refresh any programmatically created views, custom views, or adapters that depend on density (e.g., sizes from getResources.getDimension) or uiMode (night mode checks). 4. Refresh specific elements: For images/backgrounds, explicitly set view.setImageResource(R.drawable.icon) or view.setBackgroundResource(R.drawable.bg) to pick the correct density or night-qualified variant. 5. Handle WebView or special views: Manually update configuration for WebView using Resources.updateConfiguration if uiMode resets, or recreate it. The activity's Resources object is automatically updated with the new Configuration, so subsequent getResources.getDrawable calls return correct variants. However, existing View hierarchy caches old resource IDs/bitmaps, leading to stale UI unless explicitly refreshed. Official docs warn: "you are responsible for resetting any elements for which you provide alternatives." Examples include reassigning orientation-specific images; extend to uiMode/density. Avoid configChanges unless necessary (e.g., video playback); prefer activity recreation for automatic resource reloading. Use ViewModels for state preservation.
Citations:
- 1: https://developer.android.com/guide/topics/resources/runtime-changes
- 2: https://stackoverflow.com/questions/18752810/refreshing-resource-after-screen-orientation-changes
- 3: https://stackoverflow.com/questions/14441154/android-reload-layout-resource-programmatically
- 4: https://developer.android.com/reference/android/content/res/Configuration
- 5: https://issuetracker.google.com/issues/170328697
🏁 Script executed:
# Find EditorActivityKt file
fd -t f "EditorActivityKt"Repository: appdevforall/CodeOnTheGo
Length of output: 145
🏁 Script executed:
# Find EditorActivity (might be .java or .kt version)
fd -t f "EditorActivity"Repository: appdevforall/CodeOnTheGo
Length of output: 551
🏁 Script executed:
# Search for the onConfigurationChanged implementation in EditorActivity files
rg "onConfigurationChanged" --type kotlin --type java -A 15Repository: appdevforall/CodeOnTheGo
Length of output: 17453
🏁 Script executed:
rg "reapplyEditorDisplayPreferences" -A 20 --type kotlin --type javaRepository: appdevforall/CodeOnTheGo
Length of output: 4160
🏁 Script executed:
# Find CodeEditorView to check initialization
fd -t f "CodeEditorView"Repository: appdevforall/CodeOnTheGo
Length of output: 128
🏁 Script executed:
# Check configureEditorIfNeeded and initialization in CodeEditorView
rg "configureEditorIfNeeded|SchemeAndroidIDE" app/src/main/java/com/itsaky/androidide/ui/CodeEditorView.kt -A 10 -B 2Repository: appdevforall/CodeOnTheGo
Length of output: 2704
🏁 Script executed:
# Check CodeEditorView constructor/initialization for theme/density-dependent state
head -100 app/src/main/java/com/itsaky/androidide/ui/CodeEditorView.ktRepository: appdevforall/CodeOnTheGo
Length of output: 4255
🏁 Script executed:
# Check if SchemeAndroidIDE.newInstance is called again or if theme is reloaded
rg "SchemeAndroidIDE|colorScheme.*=" app/src/main/java/com/itsaky/androidide/ui/CodeEditorView.kt -B 2 -A 2Repository: appdevforall/CodeOnTheGo
Length of output: 519
🏁 Script executed:
# Check configureEditorIfNeeded full implementation and what it covers
rg "private fun configureEditorIfNeeded" app/src/main/java/com/itsaky/androidide/ui/CodeEditorView.kt -A 20Repository: appdevforall/CodeOnTheGo
Length of output: 748
🏁 Script executed:
# Search for density or uiMode related handling in CodeEditorView
rg "density|uiMode|theme|dark" app/src/main/java/com/itsaky/androidide/ui/CodeEditorView.kt -iRepository: appdevforall/CodeOnTheGo
Length of output: 50
🏁 Script executed:
# Check EditorActivityKt hierarchy and if it calls onConfigurationChanged from parent
cat app/src/main/java/com/itsaky/androidide/activities/editor/EditorActivityKt.kt | head -50Repository: appdevforall/CodeOnTheGo
Length of output: 953
🏁 Script executed:
# Check the full initialization block where colorScheme is set
rg "colorScheme = SchemeAndroidIDE" app/src/main/java/com/itsaky/androidide/ui/CodeEditorView.kt -B 10 -A 5Repository: appdevforall/CodeOnTheGo
Length of output: 548
Don't opt out of uiMode/density recreation unless those resources are fully rebound.
The onConfigurationChanged() handler only reapplies editor preferences (font size, typeface) via configureEditorIfNeeded(). It does not recreate theme-dependent state like SchemeAndroidIDE.newInstance(context) or re-apply density-dependent values initialized during CodeEditorView construction (e.g., SizeUtils.dp2px for divider width). Dark-mode or display-scale changes will leave the color scheme and density-dependent drawables stale.
The docstring in reapplyEditorDisplayPreferences() explicitly states it handles "system font scale," which is the scope of this PR. Keep fontScale and let uiMode and density trigger activity recreation until the full UI rebinding is implemented.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/main/AndroidManifest.xml` around lines 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.
| /** | ||
| * Reloads disk content into an open editor only when the file changed on disk since the last | ||
| * [onPause] snapshot **and** the in-memory buffer is still clean ([CodeEditorView.isModified] is | ||
| * false). Never replaces buffers with unsaved edits or touches undo history for dirty files. | ||
| */ | ||
| private fun checkForExternalFileChanges() { | ||
| // Get the list of files currently managed by the ViewModel | ||
| val openFiles = editorViewModel.getOpenedFiles() | ||
| if (openFiles.isEmpty() || fileTimestamps.isEmpty()) return | ||
|
|
||
| lifecycleScope.launch(Dispatchers.IO) { | ||
| // Check each open file | ||
| openFiles.forEach { file -> | ||
| val lastKnownTimestamp = fileTimestamps[file.absolutePath] ?: return@forEach | ||
| val currentTimestamp = file.lastModified() | ||
|
|
||
| // If the file on disk is newer. | ||
| if (currentTimestamp > lastKnownTimestamp) { | ||
| val newContent = runCatching { file.readText() }.getOrNull() ?: return@forEach | ||
| withContext(Dispatchers.Main) { | ||
| // If the editor for the new file exists AND has no unsaved changes... | ||
| val editorView = getEditorForFile(file) ?: return@withContext | ||
| if (editorView.isModified) return@withContext | ||
| val ideEditor = editorView.editor ?: return@withContext | ||
| if (ideEditor.canUndo() || ideEditor.canRedo()) { | ||
| return@withContext | ||
| } | ||
|
|
||
| editorView.editor?.setText(newContent) | ||
| ideEditor.setText(newContent) | ||
| editorView.markAsSaved() |
There was a problem hiding this comment.
canUndo()/canRedo() is too broad as the reload gate.
CodeEditorView.markAsSaved() only calls markUnmodified(), and IDEEditor.markUnmodified() only clears isModified (see app/src/main/java/com/itsaky/androidide/ui/CodeEditorView.kt:318-322 and editor/src/main/java/com/itsaky/androidide/editor/ui/IDEEditor.kt:662-673). After any manual save, Line 297 can still stay true, so external updates are silently ignored for a clean editor and the user keeps editing a stale buffer. If undo history must be preserved, this path needs an explicit conflict prompt instead of an early return.
🤖 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 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.
1 participant
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.