fix(terminal): use pendingCommand in onNewIntent and respect terminat… #1014
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes terminal command handling by properly using pendingCommand in the onNewIntent method and implementing support for the terminatePreviousSession flag. The changes ensure that when launching internal terminal commands, existing sessions can be terminated before creating new ones, and the session ID is consistently used from the pending command.
Key Changes:
- Added logic to check and terminate previous sessions when
terminatePreviousSessionis true - Extracted
pendingCommandto a local variable to avoid repeated null checks - Updated
onNewIntentin Terminal.kt to handle pending commands with fallback to original pwd-based behavior
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| core/main/src/main/java/com/rk/terminal/TerminalScreen.kt | Implements terminatePreviousSession logic and extracts pendingCommand to a local variable for consistent sessionId usage in the factory block |
| core/main/src/main/java/com/rk/activities/terminal/Terminal.kt | Adds pendingCommand handling in onNewIntent with terminatePreviousSession support and maintains backward compatibility with pwd-based session creation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| terminalActivity.sessionBinder?.get()!!.getSession(sessionId) != null) { | ||
| terminalActivity.sessionBinder?.get()!!.terminateSession(sessionId) | ||
| } |
There was a problem hiding this comment.
Multiple null assertion operators (!!) on sessionBinder?.get() create potential for NullPointerException. Consider extracting sessionBinder?.get() to a local variable once at the beginning of the block, similar to how pendingCommand is handled. This would improve both safety and readability by reducing repeated null assertions.
| terminalActivity.sessionBinder?.get()!!.getService().currentSession.value = | ||
| pendingCommand!!.id | ||
| terminalActivity.sessionBinder?.get()!!.getSession(pendingCommand!!.id) | ||
| sessionId | ||
| terminalActivity.sessionBinder?.get()!!.getSession(sessionId) |
There was a problem hiding this comment.
Additional calls to sessionBinder?.get()!! with null assertions. Extract sessionBinder?.get() to a local variable to avoid repeated null assertions and potential crashes.
| this@Terminal.changeSession(info.id) | ||
|
|
||
| // Check if there's a pending command from launchInternalTerminal | ||
| val command = com.rk.exec.pendingCommand |
There was a problem hiding this comment.
Reading pendingCommand without synchronization could lead to race conditions if the command is modified from another thread. Since pendingCommand is a mutable global variable, consider using proper synchronization or thread-safe mechanisms to access it, especially since this code runs in a coroutine context.
This pull request enhances how terminal sessions are managed when launching a terminal with a pending command. It introduces logic to handle session reuse and termination more robustly, ensuring that commands requesting a fresh session can terminate any previous session with the same ID before starting a new one. This improves reliability and predictability for users launching terminals with specific commands.
Session management improvements:
Terminal.kt,TerminalScreen.kt) [1] [2]Terminal.kt)Terminal.kt,TerminalScreen.kt) [1] [2]