8948962 · opencode-android

git.iko.soy/opencode-android

Fix FileBrowserViewModel state race, rotation state loss, main-thread markdown export, and fragile TimeFormat recomposition

8948962iko3 months ago

app/src/main/java/soy/iko/opencode/ui/chat/ChatScreen.kt+12−6

compare with master

unifiedsplit

@@ -82,8 +82,10 @@ import androidx.compose.ui.text.input.ImeAction

82import androidx.compose.ui.unit.dp

83import androidx.lifecycle.compose.collectAsStateWithLifecycle

84import androidx.lifecycle.viewmodel.compose.viewModel

85+import kotlinx.coroutines.Dispatchers

85import kotlinx.coroutines.flow.collectLatest

86import kotlinx.coroutines.flow.collectLatest

86import kotlinx.coroutines.launch

87import kotlinx.coroutines.launch

88+import kotlinx.coroutines.withContext

87import soy.iko.opencode.data.network.EventStreamClient

89import soy.iko.opencode.data.network.EventStreamClient

88import soy.iko.opencode.di.AppContainer

90import soy.iko.opencode.di.AppContainer

89import soy.iko.opencode.R

91import soy.iko.opencode.R

@@ -244,13 +246,17 @@ fun ChatScreen(

244 actions = {

246 actions = {

245 IconButton(

247 IconButton(

246 onClick = {

248 onClick = {

249+ scope.launch {

250+ val md = withContext(Dispatchers.Default) {

247− val md = buildConversationMarkdown(messages, sessionTitle)

251+ buildConversationMarkdown(messages, sessionTitle)

252+ }

248− val send = android.content.Intent(android.content.Intent.ACTION_SEND).apply {

253+ val send = android.content.Intent(android.content.Intent.ACTION_SEND).apply {

249− type = "text/markdown"

254+ type = "text/markdown"

250− putExtra(android.content.Intent.EXTRA_SUBJECT, sessionTitle ?: defaultShareSubject)

255+ putExtra(android.content.Intent.EXTRA_SUBJECT, sessionTitle ?: defaultShareSubject)

251− putExtra(android.content.Intent.EXTRA_TEXT, md)

256+ putExtra(android.content.Intent.EXTRA_TEXT, md)

257+ }

258+ runCatching { shareContext.startActivity(android.content.Intent.createChooser(send, shareLabel)) }

252 }

259 }

253− runCatching { shareContext.startActivity(android.content.Intent.createChooser(send, shareLabel)) }

254 },

260 },

255 enabled = messages.isNotEmpty(),

261 enabled = messages.isNotEmpty(),

256 ) {

262 ) {

app/src/main/java/soy/iko/opencode/ui/chat/PermissionDialog.kt+2−1

@@ -18,6 +18,7 @@ import androidx.compose.material3.TextButton

18import androidx.compose.runtime.Composable

19import androidx.compose.runtime.mutableStateOf

20import androidx.compose.runtime.remember

21+import androidx.compose.runtime.saveable.rememberSaveable

21import androidx.compose.runtime.getValue

22import androidx.compose.runtime.getValue

22import androidx.compose.runtime.setValue

23import androidx.compose.runtime.setValue

23import androidx.compose.ui.Modifier

24import androidx.compose.ui.Modifier

@@ -42,7 +43,7 @@ fun PermissionDialog(

42 val haptics = LocalHapticFeedback.current

43 val haptics = LocalHapticFeedback.current

43 // Guard against double-respond: back press + button tap, or rapid double-tap,

44 // Guard against double-respond: back press + button tap, or rapid double-tap,

44 // could call onRespond twice. Once a response is sent, subsequent calls are no-ops.

45 // could call onRespond twice. Once a response is sent, subsequent calls are no-ops.

45− var responded by remember { mutableStateOf(false) }

46+ var responded by rememberSaveable { mutableStateOf(false) }

46 val respond: (PermissionResponse) -> Unit = { response ->

47 val respond: (PermissionResponse) -> Unit = { response ->

47 if (!responded) {

48 if (!responded) {

48 responded = true

49 responded = true

app/src/main/java/soy/iko/opencode/ui/components/TimeFormat.kt+13−3

@@ -2,6 +2,7 @@ package soy.iko.opencode.ui.components

3import androidx.compose.runtime.Composable

4import androidx.compose.runtime.LaunchedEffect

5+import androidx.compose.runtime.derivedStateOf

5import androidx.compose.runtime.getValue

6import androidx.compose.runtime.getValue

6import androidx.compose.runtime.mutableLongStateOf

7import androidx.compose.runtime.mutableLongStateOf

7import androidx.compose.runtime.remember

8import androidx.compose.runtime.remember

@@ -59,7 +60,16 @@ fun rememberRelativeTime(epochMillis: Long?, intervalMs: Long = 30_000L): String

59 lifecycleOwner.lifecycle.removeObserver(observer)

60 lifecycleOwner.lifecycle.removeObserver(observer)

60 }

61 }

62 }

62− // Read tick so Compose tracks it as a dependency and recomposes when it changes.

63+ // derivedStateOf reads tick so Compose recomposes when it changes, and recomputes

63− @Suppress("UNUSED_EXPRESSION") tick

64+ // the relative-time string from the current wall clock on each tick.

65+ // Keying remember on epochMillis ensures a new derivedStateOf is created when the

66+ // timestamp changes, so the new value is reflected immediately.

67+ val formatted by remember(epochMillis) {

68+ derivedStateOf {

69+ // Read tick to create a recomposition dependency on the timer.

70+ tick

64− return relativeTime(epochMillis)

71+ relativeTime(epochMillis)

72+ }

73+ }

74+ return formatted

65}

75}

app/src/main/java/soy/iko/opencode/ui/file/FileBrowserViewModel.kt+15−14

@@ -16,6 +16,7 @@ import kotlinx.coroutines.flow.MutableStateFlow

16import kotlinx.coroutines.flow.StateFlow

17import kotlinx.coroutines.flow.asStateFlow

18import kotlinx.coroutines.flow.collectLatest

19+import kotlinx.coroutines.flow.update

19import kotlinx.coroutines.launch

20import kotlinx.coroutines.launch

21@Immutable

22@Immutable

@@ -76,7 +77,7 @@ class FileBrowserViewModel(private val container: AppContainer) : ViewModel() {

76 statusJob = viewModelScope.launch {

77 statusJob = viewModelScope.launch {

77 runCatchingCancellable { client.fileStatus() }

78 runCatchingCancellable { client.fileStatus() }

78 .onSuccess { entries ->

79 .onSuccess { entries ->

79− _state.value = _state.value.copy(statusMap = entries.associateBy { it.path })

80+ _state.update { it.copy(statusMap = entries.associateBy { it.path }) }

80 }

81 }

81 .onFailure {

82 .onFailure {

82 // VCS status is best-effort: don't block the file browser, but surface

83 // VCS status is best-effort: don't block the file browser, but surface

@@ -88,32 +89,32 @@ class FileBrowserViewModel(private val container: AppContainer) : ViewModel() {

89 fun open(path: String) {

90 fun open(path: String) {

90 val client = api ?: run {

91 val client = api ?: run {

91− _state.value = _state.value.copy(

92+ _state.update { it.copy(

92 path = path,

93 path = path,

93 loading = false,

94 loading = false,

94 error = container.string(R.string.not_connected),

95 error = container.string(R.string.not_connected),

95− )

96+ ) }

96 return

97 return

97 }

98 }

98− _state.value = _state.value.copy(

99+ _state.update { it.copy(

99 path = path,

100 path = path,

100 query = "",

101 query = "",

101 results = emptyList(),

102 results = emptyList(),

102 searching = false,

103 searching = false,

103 loading = true,

104 loading = true,

104 error = null,

105 error = null,

105− )

106+ ) }

106 openJob?.cancel()

107 openJob?.cancel()

107 searchJob?.cancel()

108 searchJob?.cancel()

108 openJob = viewModelScope.launch {

109 openJob = viewModelScope.launch {

109 runCatchingCancellable { client.listDirectory(path) }

110 runCatchingCancellable { client.listDirectory(path) }

110 .onSuccess { entries ->

111 .onSuccess { entries ->

111− _state.value = _state.value.copy(

112+ _state.update { it.copy(

112 entries = entries.sortedWith(compareByDescending<FileNode> { it.isDirectory }.thenBy { it.name.lowercase() }),

113 entries = entries.sortedWith(compareByDescending<FileNode> { it.isDirectory }.thenBy { it.name.lowercase() }),

113 loading = false,

114 loading = false,

114− )

115+ ) }

115 }

116 }

116− .onFailure { _state.value = _state.value.copy(loading = false, error = container.friendlyError(it)) }

117+ .onFailure { e -> _state.update { it.copy(loading = false, error = container.friendlyError(e)) } }

117 }

118 }

119 }

119

120

@@ -126,23 +127,23 @@ class FileBrowserViewModel(private val container: AppContainer) : ViewModel() {

126 }

127 }

127

128

128 fun setQuery(query: String) {

129 fun setQuery(query: String) {

129− _state.value = _state.value.copy(query = query, error = null)

130+ _state.update { it.copy(query = query, error = null) }

130 searchJob?.cancel()

131 searchJob?.cancel()

131 openJob?.cancel()

132 openJob?.cancel()

132 if (query.isBlank()) {

133 if (query.isBlank()) {

133− _state.value = _state.value.copy(results = emptyList(), searching = false)

134+ _state.update { it.copy(results = emptyList(), searching = false) }

134 return

135 return

135 }

136 }

136 val client = api ?: run {

137 val client = api ?: run {

137− _state.value = _state.value.copy(searching = false, error = container.string(R.string.not_connected))

138+ _state.update { it.copy(searching = false, error = container.string(R.string.not_connected)) }

138 return

139 return

139 }

140 }

140 searchJob = viewModelScope.launch {

141 searchJob = viewModelScope.launch {

141 delay(NetworkConfig.fileSearchDebounceMs) // debounce

142 delay(NetworkConfig.fileSearchDebounceMs) // debounce

142− _state.value = _state.value.copy(searching = true)

143+ _state.update { it.copy(searching = true) }

143 runCatchingCancellable { client.findFiles(query) }

144 runCatchingCancellable { client.findFiles(query) }

144− .onSuccess { _state.value = _state.value.copy(results = it, searching = false) }

145+ .onSuccess { results -> _state.update { it.copy(results = results, searching = false) } }

145− .onFailure { _state.value = _state.value.copy(searching = false, error = container.friendlyError(it)) }

146+ .onFailure { e -> _state.update { it.copy(searching = false, error = container.friendlyError(e)) } }

146 }

147 }

148 }

148}

149}

app/src/main/java/soy/iko/opencode/ui/session/SessionListScreen.kt+1−1

@@ -287,7 +287,7 @@ fun SessionListScreen(

287 }

288

289 pendingRename?.let { session ->

290− var title by remember(session.id) { mutableStateOf(session.title ?: "") }

290+ var title by rememberSaveable(session.id) { mutableStateOf(session.title ?: "") }

291 RenameSessionDialog(

292 title = title,

293 onTitleChange = { title = it },