Conversation
skjnldsv
requested review from
Altahrim,
come-nc,
icewind1991 and
leftybournes
and removed request for
a team
September 22, 2026 11:20
8 of 12 tasks
Split out of nextcloud#63796. Providers registered via registerProviderClosure() now keep their class name, and PreviewManager rebuilds the provider map on each getProviders() call so foreach order follows the index of each class in enabledPreviewProviders (registration order for anything not listed). Generator::generateProviderPreview() collects matches first to preserve that order, then wraps getThumbnail() in a try/catch so one throwing provider is skipped in favor of the next instead of aborting generation. When enabledPreviewProviders is unset, the computed default now prefers Imaginary first if preview_imaginary_url is configured, with native HEIC appended as a fallback when Imagick supports it. The shared default/ recommended provider lists live in the new PreviewProviderDefaults so the upcoming admin settings page (PR B) can reuse them without duplicating the provider list literals. Assisted-by: ClaudeCode:claude-sonnet-5 Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
skjnldsv
force-pushed
the
feature/preview-provider-try-order
branch
from
September 30, 2026 07:14
2739721 to
119547e
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split out of #63796 per review discussion there (splitting the combined admin-UI + provider-ordering + failures work into separate scoped PRs). This is the provider-ordering half, no UI attached. The admin settings page in #63796 will be rebased on top of this once it's split out too.
relates to #63795
enabledPreviewProviderscurrently only acts as a whitelist: order in the array doesn't matter, andPreviewManager::getProviders()sorts by regex string length. This PR makes the array order the generation try-order instead.registerProviderClosure()now keeps the registering class name, andPreviewManagerrebuilds the provider map on eachgetProviders()call so foreach order follows the index of each class inenabledPreviewProviders(registration order for anything not listed, so unconfigured instances see no behavior change).Generator::generateProviderPreview()collects matching providers into the new order first, then tries them in sequence. It also now catches\ThrowablearoundgetThumbnail()so a provider that throws (missing binary, corrupt file, whatever) is skipped in favor of the next provider instead of aborting the whole preview attempt. Flagging the broad catch for review: this used to let exceptions from providers propagate, so if anything currently relies on that propagation this is a behavior change.When
enabledPreviewProvidersis unset, the computed default now prefers Imaginary first ifpreview_imaginary_urlis configured, with native HEIC appended as a fallback when Imagick supports it, instead of always defaulting to the plain native list. Same config keys and types as before, just a smarter unset-default.The shared default/recommended provider list logic lives in a new
PreviewProviderDefaults(lib/private/Preview), pulled out so the admin settings page PR can reuse it instead of duplicating the provider list literals.👾 This pull request was assisted by Claude Code, commits carry an
Assisted-bytrailer.