-
Notifications
You must be signed in to change notification settings - Fork 15
ADFA-3133 Bug fix for over-eager auto-save during onPause event #1151
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: stage
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -232,6 +232,10 @@ open class EditorHandlerActivity : | |
| loadPluginTabs() | ||
| } | ||
|
|
||
| /** | ||
| * Persists which tabs are open (preferences only). Does **not** write project file buffers to disk; | ||
| * saving is explicit or prompted (e.g. close project). | ||
| */ | ||
| override fun onPause() { | ||
| super.onPause() | ||
| // Record timestamps for all currently open files before saving the cache | ||
|
|
@@ -246,7 +250,6 @@ open class EditorHandlerActivity : | |
| if (!isOpenedFilesSaved.get()) { | ||
| saveOpenedFiles() | ||
| saveOpenedPluginTabs() | ||
| saveAllAsync(notify = false) | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -271,26 +274,31 @@ open class EditorHandlerActivity : | |
| invalidateOptionsMenu() | ||
| } | ||
|
|
||
| /** | ||
| * 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() | ||
|
Comment on lines
+277
to
302
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
🤖 Prompt for AI Agents |
||
| updateTabs() | ||
| } | ||
|
|
@@ -341,12 +349,19 @@ open class EditorHandlerActivity : | |
| prefs.getString(PREF_KEY_OPEN_FILES_CACHE, null) | ||
| } ?: return@launch | ||
|
|
||
| if (editorViewModel.getOpenedFileCount() > 0) { | ||
| // Returning to an in-memory session (e.g. after onPause/onStop). Replaying the | ||
| // snapshot would be redundant and could interfere with dirty buffers and undo. | ||
| withContext(Dispatchers.IO) { prefs.putString(PREF_KEY_OPEN_FILES_CACHE, null) } | ||
| return@launch | ||
| } | ||
|
|
||
| val cache = withContext(Dispatchers.Default) { | ||
| Gson().fromJson(jsonCache, OpenedFilesCache::class.java) | ||
| } | ||
| onReadOpenedFilesCache(cache) | ||
|
|
||
| // Clear the preference so it's only loaded once on startup | ||
| // Clear the preference so it's only loaded once per cold restore | ||
| withContext(Dispatchers.IO) { prefs.putString(PREF_KEY_OPEN_FILES_CACHE, null) } | ||
| } catch (err: Throwable) { | ||
| log.error("Failed to reopen recently opened files", err) | ||
|
|
@@ -747,6 +762,11 @@ open class EditorHandlerActivity : | |
| override fun onConfigurationChanged(newConfig: Configuration) { | ||
| super.onConfigurationChanged(newConfig) | ||
|
|
||
| val safeContent = contentOrNull ?: return | ||
| for (i in 0 until safeContent.editorContainer.childCount) { | ||
| (safeContent.editorContainer.getChildAt(i) as? CodeEditorView)?.reapplyEditorDisplayPreferences() | ||
| } | ||
|
|
||
| getCurrentEditor()?.editor?.apply { | ||
| doOnNextLayout { | ||
| cursor?.let { c -> ensurePositionVisible(c.leftLine, c.leftColumn, true) } | ||
|
|
@@ -1069,17 +1089,20 @@ open class EditorHandlerActivity : | |
| nameBuilder.addPath(it, it.path) | ||
| } | ||
|
|
||
| for (index in 0 until content.tabs.tabCount) { | ||
| val file = files.getOrNull(index) ?: continue | ||
| for (tabPos in 0 until content.tabs.tabCount) { | ||
| if (isPluginTab(tabPos)) continue | ||
| val fileIndex = getFileIndexForTabPosition(tabPos) | ||
| if (fileIndex < 0) continue | ||
| val file = files.getOrNull(fileIndex) ?: continue | ||
| val count = dupliCount[file.name] ?: 0 | ||
|
|
||
| val isModified = getEditorAtIndex(index)?.isModified ?: false | ||
| val isModified = getEditorAtIndex(fileIndex)?.isModified ?: false | ||
| var name = if (count > 1) nameBuilder.getShortPath(file) else file.name | ||
| if (isModified) { | ||
| name = "*$name" | ||
| } | ||
|
|
||
| names[index] = name to FileExtension.Factory.forFile(file, file.isDirectory).icon | ||
| names[tabPos] = name to FileExtension.Factory.forFile(file, file.isDirectory).icon | ||
| } | ||
|
|
||
| withContext(Dispatchers.Main) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
When an Android activity handlesuiModeanddensityviaandroid:configChanges, what theme/resource updates must it perform manually inonConfigurationChangedto 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:
🏁 Script executed:
Repository: appdevforall/CodeOnTheGo
Length of output: 145
🏁 Script executed:
Repository: appdevforall/CodeOnTheGo
Length of output: 551
🏁 Script executed:
Repository: appdevforall/CodeOnTheGo
Length of output: 17453
🏁 Script executed:
rg "reapplyEditorDisplayPreferences" -A 20 --type kotlin --type javaRepository: appdevforall/CodeOnTheGo
Length of output: 4160
🏁 Script executed:
Repository: appdevforall/CodeOnTheGo
Length of output: 128
🏁 Script executed:
Repository: 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:
Repository: appdevforall/CodeOnTheGo
Length of output: 519
🏁 Script executed:
Repository: appdevforall/CodeOnTheGo
Length of output: 748
🏁 Script executed:
Repository: appdevforall/CodeOnTheGo
Length of output: 50
🏁 Script executed:
Repository: appdevforall/CodeOnTheGo
Length of output: 953
🏁 Script executed:
Repository: appdevforall/CodeOnTheGo
Length of output: 548
Don't opt out of
uiMode/densityrecreation unless those resources are fully rebound.The
onConfigurationChanged()handler only reapplies editor preferences (font size, typeface) viaconfigureEditorIfNeeded(). It does not recreate theme-dependent state likeSchemeAndroidIDE.newInstance(context)or re-apply density-dependent values initialized duringCodeEditorViewconstruction (e.g.,SizeUtils.dp2pxfor 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. KeepfontScaleand letuiModeanddensitytrigger activity recreation until the full UI rebinding is implemented.🤖 Prompt for AI Agents