Conversation
There was a problem hiding this comment.
Pull request overview
Refactors crash-log collection so the crash screen can display logs via a lazily-started StateFlow instead of an explicit UI-triggered loadLogs() call.
Changes:
- Removed the standalone
dumpCrash()helper and inlined logcat dumping intoAppCrashedViewModel. - Reworked crash log loading to be driven by a
flow { ... }.stateIn(...)pipeline. - Simplified
AppCrashedScreenby removingLaunchedEffect-based manual loading.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| app/src/main/kotlin/com/github/kr328/clash/log/util/SystemLogcat.kt | Deletes the old logcat-dump utility (functionality moved). |
| app/src/main/kotlin/com/github/kr328/clash/crash/vm/AppCrashedViewModel.kt | Implements lazy crash-log loading via StateFlow and adds a local dumpCrash() using Runtime.exec. |
| app/src/main/kotlin/com/github/kr328/clash/crash/ui/AppCrashedScreen.kt | Removes the manual loadLogs() trigger and relies on collecting the StateFlow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ViewModel.kt Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ViewModel.kt Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| val process = ProcessBuilder(*crashDumpCommand).redirectErrorStream(true).start() | ||
| val result = | ||
| process.inputStream | ||
| .bufferedReader() | ||
| .readLines() | ||
| .filterNot { it.startsWith("------") } | ||
| .joinToString("\n") | ||
| val exitCode = process.waitFor() | ||
|
|
||
| if (exitCode != 0) { | ||
| error("logcat exited with code $exitCode: ${result.trim()}") | ||
| } | ||
| result.trim() | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
process.inputStream...useLines { ... } and process.waitFor() are both unbounded waits. If logcat -d stalls/hangs on a device, the crash screen will never receive an emission (it will stay at the initial "" state). Consider enforcing a timeout (e.g., waitFor(timeout, ...)) and ensuring the process is destroyed in finally on timeout/exception so the UI can always proceed.
No description provided.