JMR-dev/LibreMediaConverter
1
0
Fork 0
Code Issues 16 Pull Requests Actions Packages Projects Releases Wiki Activity

Two of the theme colour-scheme branches are unreachable, and the KDoc promises a switch that does not exist #68

New Issue
Open
opened 2026-08-24 21:01:24 +00:00 by JMR-dev · 0 comments
JMR-dev commented 2026-08-24 21:01:24 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Named as out-of-scope in #52 so its absence would be a decision rather than an oversight. Looking at it properly turned up more than missing coverage.

Two of the four colour-scheme branches cannot be reached

app/src/main/java/org/libremediaconverter/ui/theme/Theme.kt:

fun LibreMediaConverterTheme(
    darkTheme: Boolean = isSystemInDarkTheme(),
    dynamicColor: Boolean = true,
    content: @Composable () -> Unit,
) {
    val colorScheme = when {
        dynamicColor && darkTheme -> dynamicDarkColorScheme(context)
        dynamicColor              -> dynamicLightColorScheme(context)
        darkTheme                 -> DarkColorScheme   // unreachable
        else                      -> LightColorScheme  // unreachable
    }

There is exactly one caller — MainActivity.kt:77, LibreMediaConverterTheme { ... }, with no
arguments. So dynamicColor is always true, the first two branches always win, and the last two
are dead. dynamicColor is never passed by anything in app/src (grep: only its own declaration
and the two branches that read it).

Consequently DarkColorScheme, LightColorScheme and all six brand colours are dead code —
Purple80, Purple40, PurpleGrey80, PurpleGrey40, Pink80, Pink40 in Color.kt have no
reference outside ui/theme/. They are the Android Studio template's palette, never replaced.

The KDoc asserts a capability that does not exist

Dynamic color (Material You) needs API 31+; minSdk is 33, so it is available unconditionally and
no version guard is required. It stays switchable so users can opt back to the brand palette.

The first sentence is correct. The second is false — there is no switch, no setting, and no
caller that could flip it. Same class as the stale-claim findings R14, R15 and R25.

The decision this needs

Not a code change to make blind — pick one:

  • Add the toggle. The parameter is already there; it needs a setting and somewhere to put it.
    Makes the KDoc true and the branches live. Largest option, and a product call.
  • Delete the dead branches and the brand palette with them, and correct the KDoc to say dynamic
    colour is unconditional. Smallest, honest, loses a capability nobody can currently use.
  • Replace the template palette with a real brand one and then decide. The current colours are
    Android Studio's defaults, so "opt back to the brand palette" does not yet mean anything.

Marked Backlog because that choice is yours, not something to guess at.

Coverage, which is how this was found

ThemeKt is 23 lines, 0 covered (jacoco on 1779f20). Whichever option wins, the surviving
branches are unit-testable today: Robolectric plus createDrainedComposeRule() can assert the
resolved scheme differs across darkTheme/dynamicColor, reading MaterialTheme.colorScheme
inside the content lambda. Not filed under #52 because that issue is the three screens.

Mutation, whichever way it goes: swap dynamicDarkColorScheme for dynamicLightColorScheme in
the first branch and the dark-mode test must go red. If it does not, the test is reading a scheme
the theme did not choose.

_Named as out-of-scope in #52 so its absence would be a decision rather than an oversight. Looking at it properly turned up more than missing coverage._ ### Two of the four colour-scheme branches cannot be reached `app/src/main/java/org/libremediaconverter/ui/theme/Theme.kt`: ```kotlin fun LibreMediaConverterTheme( darkTheme: Boolean = isSystemInDarkTheme(), dynamicColor: Boolean = true, content: @Composable () -> Unit, ) { val colorScheme = when { dynamicColor && darkTheme -> dynamicDarkColorScheme(context) dynamicColor -> dynamicLightColorScheme(context) darkTheme -> DarkColorScheme // unreachable else -> LightColorScheme // unreachable } ``` **There is exactly one caller** — `MainActivity.kt:77`, `LibreMediaConverterTheme { ... }`, with no arguments. So `dynamicColor` is always `true`, the first two branches always win, and the last two are dead. `dynamicColor` is never passed by anything in `app/src` (grep: only its own declaration and the two branches that read it). Consequently **`DarkColorScheme`, `LightColorScheme` and all six brand colours are dead code** — `Purple80`, `Purple40`, `PurpleGrey80`, `PurpleGrey40`, `Pink80`, `Pink40` in `Color.kt` have no reference outside `ui/theme/`. They are the Android Studio template's palette, never replaced. ### The KDoc asserts a capability that does not exist > Dynamic color (Material You) needs API 31+; minSdk is 33, so it is available unconditionally and > no version guard is required. **It stays switchable so users can opt back to the brand palette.** The first sentence is correct. **The second is false** — there is no switch, no setting, and no caller that could flip it. Same class as the stale-claim findings R14, R15 and R25. ### The decision this needs Not a code change to make blind — pick one: - **Add the toggle.** The parameter is already there; it needs a setting and somewhere to put it. Makes the KDoc true and the branches live. Largest option, and a product call. - **Delete the dead branches** and the brand palette with them, and correct the KDoc to say dynamic colour is unconditional. Smallest, honest, loses a capability nobody can currently use. - **Replace the template palette** with a real brand one and then decide. The current colours are Android Studio's defaults, so "opt back to the brand palette" does not yet mean anything. Marked **Backlog** because that choice is yours, not something to guess at. ### Coverage, which is how this was found `ThemeKt` is 23 lines, **0 covered** (jacoco on `1779f20`). Whichever option wins, the surviving branches are unit-testable today: Robolectric plus `createDrainedComposeRule()` can assert the resolved scheme differs across `darkTheme`/`dynamicColor`, reading `MaterialTheme.colorScheme` inside the content lambda. Not filed under #52 because that issue is the three screens. **Mutation, whichever way it goes:** swap `dynamicDarkColorScheme` for `dynamicLightColorScheme` in the first branch and the dark-mode test must go red. If it does not, the test is reading a scheme the theme did not choose.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
above-cut
Worked autonomously overnight: local, JVM-verifiable, no product decision
 accessibility
Barrier affecting people with disabilities
 backlog
Held for manual review: product/UX call, CI/workflow, hardware, or unverifiable here
 bug
Something isn't working
 confirmed
Reviewer demonstrated the defect
 documentation
Improvements or additions to documentation
 duplicate
This issue or pull request already exists
 enhancement
New feature or request
 good first issue
Good for newcomers
 help wanted
Extra attention is needed
 invalid
This doesn't seem right
 plausible
Reviewer could not fully demonstrate it; treat as unproven
 question
Further information is requested
 sev:high
High severity
 sev:low
Low severity
 sev:medium
Medium severity
 wontfix
This will not be worked on
No labels bug confirmed sev:low
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
JMR-dev
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: JMR-dev/LibreMediaConverter#68