Skip to content

refactor(legacy): simplify getTheme and clarify legacy theme docs#60481

Merged
come-nc merged 1 commit into
masterfrom
jtr/legacy-getTheme
Jun 4, 2026
Merged

refactor(legacy): simplify getTheme and clarify legacy theme docs#60481
come-nc merged 1 commit into
masterfrom
jtr/legacy-getTheme

Conversation

@joshtrichards

@joshtrichards joshtrichards commented May 16, 2026

Copy link
Copy Markdown
Member

Summary

Clean up OC_Util::getTheme() and clarify its PHPDoc.

Details

  • simplify control flow with early returns
  • rename the local variable for readability
  • document that this only applies to legacy filesystem-based themes
  • document that configured theme directories are not validated
  • clarify the possible return values

No behavior change intended.

Notes

  • The default fallback theme doesn't exist in any modern installation so most of the time this is just going to return an empty string (this is why I was looking at this function -- the places we call it from, such as in URLGenenerator::imagePath() tend not to check for an empty return value... leading to unnecessary work in a few spots).
  • We can't get rid of this legacy tiny helper outright just yet (though maybe we can/should move somewhere more appropriate and/or rename it).

Checklist

AI (if applicable)

  • The content of this PR was partly or fully generated using AI

Signed-off-by: Josh <josh.t.richards@gmail.com>
@joshtrichards joshtrichards requested a review from a team as a code owner May 16, 2026 14:16
@joshtrichards joshtrichards requested review from ArtificialOwl, artonge, come-nc and icewind1991 and removed request for a team May 16, 2026 14:16
@joshtrichards joshtrichards added this to the Nextcloud 35 milestone May 16, 2026
@come-nc come-nc merged commit d4c8781 into master Jun 4, 2026
230 of 246 checks passed
@come-nc come-nc deleted the jtr/legacy-getTheme branch June 4, 2026 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews feature: theming technical debt 🧱 🤔🚀

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants