mirror of
https://github.com/KRTirtho/spotube.git
synced 2026-11-04 10:13:59 +00:00
Merge pull request #3203 from team-spotube/fix/linux-string-lookup-segfault
fix(desktop): stop non-composable string lookups from querying the system theme
This commit is contained in:
commit
0bf0e1816c
@ -54,6 +54,7 @@
|
||||
|
||||
## UI component patterns
|
||||
- **ALL user-facing strings MUST come from `composeApp/src/commonMain/composeResources/values/strings.xml` via `stringResource(Res.string.xxx)`.** Never hardcode a literal string in a `Text(...)`, dialog title, button label, content description, placeholder, or error message inside `@Composable` code. If the string you need doesn't exist yet, add a new `<string name="...">` entry to `strings.xml` (grouped near related keys, following the existing `feature_context_purpose` naming convention) and reference it — do not inline the text "just this once". This applies to every module, including ones you're about to add (dialogs, screens, snackbars, etc.), not just `modules/settings`. `strings.xml` is the single source of truth for copy, not a showcase file for one screen.
|
||||
- Outside composables (ViewModels, services, snackbars), import `getString` / `getPluralString` / `getStringArray` from `org.spotube.spotube.core.i18n`, never from `org.jetbrains.compose.resources`. The library versions ask Skiko for the system theme on every call, which on Linux is a blocking D-Bus call that segfaulted the desktop app from background threads (#3183). `NonComposableStringLookupTest` enforces this.
|
||||
- **ViewModels own ALL state and logic.** Composables are dumb renderers: they collect a single `StateFlow<UiState>` from the ViewModel and forward user events (clicks, text input, drag callbacks) back to ViewModel functions. No business logic, filtering, derivation, reordering buffers, or `LaunchedEffect`-based state syncing belongs in a composable — it goes in the ViewModel. The `combine`/`stateIn` flow chain in the ViewModel must produce fully-computed, ready-to-render UI state so the composable never needs intermediate `remember` derivations or `mutableStateListOf` mirrors.
|
||||
- **AdaptiveDropdownBottomSheet** (`commonMain/.../core/ui/component/AdaptiveDropdownBottomSheet.kt`): switches between `DropdownMenu` (large screen) and `ModalBottomSheet` (small screen) via `currentWindowAdaptiveInfo()`. Do NOT use expect/actual — all adaptive components that rely ONLY on Compose/Material3 APIs belong in commonMain.
|
||||
- **AdaptiveDialogBottomSheet** (`commonMain/.../core/ui/component/AdaptiveDialogBottomSheet.kt`): switches between `ThemedDialog` (large screen) and `ModalBottomSheet` (small screen) via `currentWindowAdaptiveInfo()`. Same rule — keep in commonMain unless platform-specific APIs are required.
|
||||
|
||||
@ -41,7 +41,7 @@ import org.spotube.spotube.modules.saved_tracks.SavedTracksRepository
|
||||
import org.spotube.spotube.modules.search.SearchRepository
|
||||
import org.spotube.spotube.modules.settings.SettingsRepository
|
||||
import kotlinx.coroutines.flow.first
|
||||
import org.jetbrains.compose.resources.getString
|
||||
import org.spotube.spotube.core.i18n.getString
|
||||
import spotube.composeapp.generated.resources.Res
|
||||
import spotube.composeapp.generated.resources.library_tab_albums
|
||||
import spotube.composeapp.generated.resources.library_tab_artists
|
||||
|
||||
@ -55,7 +55,7 @@ import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.withContext
|
||||
import org.koin.core.component.KoinComponent
|
||||
import org.koin.core.component.inject
|
||||
import org.jetbrains.compose.resources.getString
|
||||
import org.spotube.spotube.core.i18n.getString
|
||||
import spotube.composeapp.generated.resources.Res
|
||||
import spotube.composeapp.generated.resources.notification_playback_channel_description
|
||||
import spotube.composeapp.generated.resources.notification_playback_channel_name
|
||||
|
||||
@ -54,6 +54,7 @@ fun AppLocaleProvider(language: SupportedLanguages, content: @Composable () -> U
|
||||
layoutDirection?.let { LocalLayoutDirection provides it })
|
||||
CompositionLocalProvider(*locales.toTypedArray()) {
|
||||
key(languageTag) {
|
||||
PublishResourceEnvironment()
|
||||
content()
|
||||
}
|
||||
}
|
||||
|
||||
@ -0,0 +1,71 @@
|
||||
/*
|
||||
* Copyright (C) 2026 Kingkor Roy Tirtho and Spotube Contributors
|
||||
*
|
||||
* This program is free software: you can redistribute it and/or modify
|
||||
* it under the terms of the GNU Affero General Public License as published by
|
||||
* the Free Software Foundation, either version 3 of the License, or
|
||||
* (at your option) any later version.
|
||||
*
|
||||
* This program is distributed in the hope that it will be useful,
|
||||
* but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
* GNU Affero General Public License for more details.
|
||||
*
|
||||
* You should have received a copy of the GNU Affero General Public License
|
||||
* along with this program. If not, see <https://www.gnu.org/licenses/>.
|
||||
*/
|
||||
|
||||
package org.spotube.spotube.core.i18n
|
||||
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.SideEffect
|
||||
import org.jetbrains.compose.resources.PluralStringResource
|
||||
import org.jetbrains.compose.resources.ResourceEnvironment
|
||||
import org.jetbrains.compose.resources.StringArrayResource
|
||||
import org.jetbrains.compose.resources.StringResource
|
||||
import org.jetbrains.compose.resources.getSystemResourceEnvironment
|
||||
import org.jetbrains.compose.resources.rememberResourceEnvironment
|
||||
import kotlin.concurrent.Volatile
|
||||
import org.jetbrains.compose.resources.getPluralString as composeGetPluralString
|
||||
import org.jetbrains.compose.resources.getString as composeGetString
|
||||
import org.jetbrains.compose.resources.getStringArray as composeGetStringArray
|
||||
|
||||
/*
|
||||
* Non-composable string lookups (ViewModels, services, snackbars). Use these instead of
|
||||
* org.jetbrains.compose.resources.getString/getPluralString/getStringArray.
|
||||
*
|
||||
* The library versions call getSystemResourceEnvironment() on every lookup. On desktop that
|
||||
* also asks Skiko for the system theme, and on Linux Skiko answers with a blocking D-Bus call to
|
||||
* the desktop portal on whatever thread is asking. Run from a background coroutine, that call
|
||||
* segfaulted the GraalVM build on GNOME (#3183). Strings never depend on the theme, so these
|
||||
* reuse the environment the UI already computed (on the UI thread, for stringResource) and only
|
||||
* fall back to the library's lookup before anything has been composed.
|
||||
*/
|
||||
|
||||
@Volatile
|
||||
private var composedEnvironment: ResourceEnvironment? = null
|
||||
|
||||
/** Publishes the UI's resource environment for [getString] and friends. Called by [AppLocaleProvider]. */
|
||||
@Composable
|
||||
internal fun PublishResourceEnvironment() {
|
||||
val environment = rememberResourceEnvironment()
|
||||
SideEffect { composedEnvironment = environment }
|
||||
}
|
||||
|
||||
private fun resourceEnvironment(): ResourceEnvironment =
|
||||
composedEnvironment ?: getSystemResourceEnvironment()
|
||||
|
||||
suspend fun getString(resource: StringResource): String =
|
||||
composeGetString(resourceEnvironment(), resource)
|
||||
|
||||
suspend fun getString(resource: StringResource, vararg formatArgs: Any): String =
|
||||
composeGetString(resourceEnvironment(), resource, *formatArgs)
|
||||
|
||||
suspend fun getPluralString(resource: PluralStringResource, quantity: Int): String =
|
||||
composeGetPluralString(resourceEnvironment(), resource, quantity)
|
||||
|
||||
suspend fun getPluralString(resource: PluralStringResource, quantity: Int, vararg formatArgs: Any): String =
|
||||
composeGetPluralString(resourceEnvironment(), resource, quantity, *formatArgs)
|
||||
|
||||
suspend fun getStringArray(resource: StringArrayResource): List<String> =
|
||||
composeGetStringArray(resourceEnvironment(), resource)
|
||||
@ -18,8 +18,8 @@
|
||||
package org.spotube.spotube.core.jam
|
||||
|
||||
import okio.ByteString.Companion.encodeUtf8
|
||||
import org.jetbrains.compose.resources.getString
|
||||
import org.jetbrains.compose.resources.getStringArray
|
||||
import org.spotube.spotube.core.i18n.getString
|
||||
import org.spotube.spotube.core.i18n.getStringArray
|
||||
import spotube.composeapp.generated.resources.Res
|
||||
import spotube.composeapp.generated.resources.jam_name_adjectives
|
||||
import spotube.composeapp.generated.resources.jam_name_animals
|
||||
|
||||
@ -58,7 +58,7 @@ import kotlinx.serialization.SerialName
|
||||
import kotlinx.serialization.Serializable
|
||||
import kotlinx.serialization.json.Json
|
||||
import kotlinx.serialization.json.JsonClassDiscriminator
|
||||
import org.jetbrains.compose.resources.getString
|
||||
import org.spotube.spotube.core.i18n.getString
|
||||
import org.spotube.spotube.core.network.HttpClientFactory
|
||||
import spotube.composeapp.generated.resources.Res
|
||||
import spotube.composeapp.generated.resources.jam_error_banned
|
||||
|
||||
@ -59,7 +59,7 @@ import kotlinx.coroutines.sync.Mutex
|
||||
import kotlinx.coroutines.sync.withLock
|
||||
import kotlinx.coroutines.withTimeoutOrNull
|
||||
import kotlin.math.abs
|
||||
import org.jetbrains.compose.resources.getString
|
||||
import org.spotube.spotube.core.i18n.getString
|
||||
import spotube.composeapp.generated.resources.Res
|
||||
import spotube.composeapp.generated.resources.jam_error_invalid_code
|
||||
import spotube.composeapp.generated.resources.jam_error_no_server
|
||||
|
||||
@ -40,8 +40,8 @@ import kotlinx.coroutines.flow.map
|
||||
import kotlinx.coroutines.flow.update
|
||||
import kotlinx.coroutines.launch
|
||||
import org.koin.core.component.KoinComponent
|
||||
import org.jetbrains.compose.resources.getPluralString
|
||||
import org.jetbrains.compose.resources.getString
|
||||
import org.spotube.spotube.core.i18n.getPluralString
|
||||
import org.spotube.spotube.core.i18n.getString
|
||||
import spotube.composeapp.generated.resources.Res
|
||||
import spotube.composeapp.generated.resources.jam_queue_added_track
|
||||
import spotube.composeapp.generated.resources.jam_queue_added_tracks
|
||||
|
||||
@ -51,7 +51,7 @@ import kotlinx.coroutines.flow.stateIn
|
||||
import kotlinx.coroutines.flow.update
|
||||
import kotlinx.coroutines.launch
|
||||
import org.koin.core.component.KoinComponent
|
||||
import org.jetbrains.compose.resources.getString
|
||||
import org.spotube.spotube.core.i18n.getString
|
||||
import spotube.composeapp.generated.resources.Res
|
||||
import spotube.composeapp.generated.resources.common_unknown_error
|
||||
|
||||
|
||||
@ -48,7 +48,7 @@ import kotlinx.coroutines.flow.stateIn
|
||||
import kotlinx.coroutines.flow.update
|
||||
import kotlinx.coroutines.launch
|
||||
import org.koin.core.component.KoinComponent
|
||||
import org.jetbrains.compose.resources.getString
|
||||
import org.spotube.spotube.core.i18n.getString
|
||||
import spotube.composeapp.generated.resources.*
|
||||
|
||||
sealed interface ArtistScreenState {
|
||||
|
||||
@ -62,7 +62,7 @@ import okio.SYSTEM
|
||||
import okio.buffer
|
||||
import okio.use
|
||||
import kotlin.time.Clock
|
||||
import org.jetbrains.compose.resources.getString
|
||||
import org.spotube.spotube.core.i18n.getString
|
||||
import spotube.composeapp.generated.resources.*
|
||||
|
||||
data class DownloadItem(
|
||||
|
||||
@ -35,7 +35,7 @@ import kotlinx.coroutines.flow.filterNotNull
|
||||
import kotlinx.coroutines.flow.flatMapLatest
|
||||
import kotlinx.coroutines.launch
|
||||
import org.koin.core.component.KoinComponent
|
||||
import org.jetbrains.compose.resources.getString
|
||||
import org.spotube.spotube.core.i18n.getString
|
||||
import spotube.composeapp.generated.resources.Res
|
||||
import spotube.composeapp.generated.resources.common_unknown_error
|
||||
|
||||
|
||||
@ -37,7 +37,7 @@ import kotlinx.coroutines.flow.stateIn
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.coroutines.delay
|
||||
import kotlinx.coroutines.Job
|
||||
import org.jetbrains.compose.resources.getString
|
||||
import org.spotube.spotube.core.i18n.getString
|
||||
import spotube.composeapp.generated.resources.*
|
||||
|
||||
enum class JamSetupMode { Create, Join }
|
||||
|
||||
@ -36,7 +36,7 @@ import kotlinx.coroutines.flow.SharingStarted
|
||||
import kotlinx.coroutines.flow.stateIn
|
||||
import kotlinx.coroutines.launch
|
||||
import org.koin.core.component.KoinComponent
|
||||
import org.jetbrains.compose.resources.getString
|
||||
import org.spotube.spotube.core.i18n.getString
|
||||
import spotube.composeapp.generated.resources.Res
|
||||
import spotube.composeapp.generated.resources.common_unknown_error
|
||||
|
||||
|
||||
@ -36,7 +36,7 @@ import kotlinx.coroutines.flow.SharingStarted
|
||||
import kotlinx.coroutines.flow.stateIn
|
||||
import kotlinx.coroutines.launch
|
||||
import org.koin.core.component.KoinComponent
|
||||
import org.jetbrains.compose.resources.getString
|
||||
import org.spotube.spotube.core.i18n.getString
|
||||
import spotube.composeapp.generated.resources.Res
|
||||
import spotube.composeapp.generated.resources.common_unknown_error
|
||||
|
||||
|
||||
@ -34,7 +34,7 @@ import kotlinx.coroutines.launch
|
||||
import kotlin.coroutines.cancellation.CancellationException
|
||||
import org.spotube.spotube.PlatformType
|
||||
import org.spotube.spotube.getPlatform
|
||||
import org.jetbrains.compose.resources.getString
|
||||
import org.spotube.spotube.core.i18n.getString
|
||||
import spotube.composeapp.generated.resources.Res
|
||||
import spotube.composeapp.generated.resources.library_local_refresh_failed
|
||||
|
||||
|
||||
@ -37,7 +37,7 @@ import kotlinx.coroutines.flow.stateIn
|
||||
import kotlinx.coroutines.launch
|
||||
import io.github.vinceglb.filekit.PlatformFile
|
||||
import org.koin.core.component.KoinComponent
|
||||
import org.jetbrains.compose.resources.getString
|
||||
import org.spotube.spotube.core.i18n.getString
|
||||
import spotube.composeapp.generated.resources.Res
|
||||
import spotube.composeapp.generated.resources.common_unknown_error
|
||||
|
||||
|
||||
@ -56,7 +56,7 @@ import org.spotube.spotube.modules.library.playlist.PlaylistFormUiState
|
||||
import kotlinx.coroutines.flow.update
|
||||
import kotlinx.coroutines.launch
|
||||
import org.koin.core.component.KoinComponent
|
||||
import org.jetbrains.compose.resources.getString
|
||||
import org.spotube.spotube.core.i18n.getString
|
||||
import spotube.composeapp.generated.resources.Res
|
||||
import spotube.composeapp.generated.resources.common_unknown_error
|
||||
|
||||
|
||||
@ -61,7 +61,7 @@ import okio.Path.Companion.toPath
|
||||
import okio.SYSTEM
|
||||
import okio.buffer
|
||||
import okio.use
|
||||
import org.jetbrains.compose.resources.getString
|
||||
import org.spotube.spotube.core.i18n.getString
|
||||
import org.koin.core.component.KoinComponent
|
||||
import spotube.composeapp.generated.resources.Res
|
||||
import spotube.composeapp.generated.resources.plugin_action_install
|
||||
|
||||
@ -59,7 +59,7 @@ import kotlinx.coroutines.flow.update
|
||||
import kotlinx.coroutines.launch
|
||||
import org.koin.compose.koinInject
|
||||
import org.koin.core.component.KoinComponent
|
||||
import org.jetbrains.compose.resources.getString
|
||||
import org.spotube.spotube.core.i18n.getString
|
||||
import spotube.composeapp.generated.resources.*
|
||||
|
||||
const val SAVED_TRACKS_COLLECTION_ID = "saved_tracks"
|
||||
|
||||
@ -34,7 +34,7 @@ import kotlinx.coroutines.SupervisorJob
|
||||
import kotlinx.coroutines.flow.distinctUntilChanged
|
||||
import kotlinx.coroutines.flow.filterNotNull
|
||||
import kotlinx.coroutines.launch
|
||||
import org.jetbrains.compose.resources.getString
|
||||
import org.spotube.spotube.core.i18n.getString
|
||||
import spotube.composeapp.generated.resources.Res
|
||||
import spotube.composeapp.generated.resources.search_error_no_metadata_plugin
|
||||
|
||||
|
||||
@ -44,7 +44,7 @@ import kotlinx.coroutines.flow.asStateFlow
|
||||
import kotlinx.coroutines.joinAll
|
||||
import kotlinx.coroutines.launch
|
||||
import kotlinx.serialization.json.Json
|
||||
import org.jetbrains.compose.resources.getString
|
||||
import org.spotube.spotube.core.i18n.getString
|
||||
import spotube.composeapp.generated.resources.Res
|
||||
import spotube.composeapp.generated.resources.search_error_capabilities
|
||||
import spotube.composeapp.generated.resources.search_error_search
|
||||
|
||||
@ -123,7 +123,7 @@ import org.koin.compose.koinInject
|
||||
import org.koin.compose.viewmodel.koinViewModel
|
||||
import org.koin.core.parameter.parametersOf
|
||||
import kotlin.time.Duration.Companion.milliseconds
|
||||
import org.jetbrains.compose.resources.getString
|
||||
import org.spotube.spotube.core.i18n.getString
|
||||
import org.jetbrains.compose.resources.stringResource
|
||||
import spotube.composeapp.generated.resources.Res
|
||||
import spotube.composeapp.generated.resources.downloads_status_cancelled
|
||||
|
||||
@ -28,7 +28,7 @@ import androidx.compose.ui.unit.LayoutDirection
|
||||
import androidx.compose.ui.use
|
||||
import org.spotube.spotube.modules.settings.SupportedLanguages
|
||||
import kotlinx.coroutines.runBlocking
|
||||
import org.jetbrains.compose.resources.getString
|
||||
import org.spotube.spotube.core.i18n.getString
|
||||
import org.jetbrains.compose.resources.pluralStringResource
|
||||
import org.jetbrains.compose.resources.stringResource
|
||||
import spotube.composeapp.generated.resources.Res
|
||||
|
||||
@ -0,0 +1,46 @@
|
||||
/*
|
||||
* Copyright (C) 2026 Kingkor Roy Tirtho and Spotube Contributors
|
||||
*
|
||||
* This program is free software: you can redistribute it and/or modify
|
||||
* it under the terms of the GNU Affero General Public License as published by
|
||||
* the Free Software Foundation, either version 3 of the License, or
|
||||
* (at your option) any later version.
|
||||
*
|
||||
* This program is distributed in the hope that it will be useful,
|
||||
* but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
* GNU Affero General Public License for more details.
|
||||
*
|
||||
* You should have received a copy of the GNU Affero General Public License
|
||||
* along with this program. If not, see <https://www.gnu.org/licenses/>.
|
||||
*/
|
||||
|
||||
package org.spotube.spotube.core.i18n
|
||||
|
||||
import java.io.File
|
||||
import kotlin.test.Test
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
/**
|
||||
* The library's non-composable getters query the system theme over D-Bus on Linux on every
|
||||
* call, which crashed the desktop app (#3183). App code must use the ones in Strings.kt.
|
||||
*/
|
||||
class NonComposableStringLookupTest {
|
||||
@Test
|
||||
fun appCodeUsesTheAppStringLookups() {
|
||||
val forbidden = Regex("""^import org\.jetbrains\.compose\.resources\.get(String|PluralString|StringArray)\s*$""")
|
||||
val offenders = File("src").walkTopDown()
|
||||
.filter { it.isFile && it.extension == "kt" }
|
||||
.flatMap { file ->
|
||||
file.readLines().withIndex()
|
||||
.filter { (_, line) -> forbidden.matches(line) }
|
||||
.map { (index, _) -> "${file.path}:${index + 1}" }
|
||||
}
|
||||
.toList()
|
||||
|
||||
assertTrue(
|
||||
offenders.isEmpty(),
|
||||
"Import getString/getPluralString/getStringArray from org.spotube.spotube.core.i18n instead:\n" +
|
||||
offenders.joinToString("\n"))
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue
Block a user