Skip to content

fix(preview): skip thumbnail generation when image preview is disabled - #1621

Open
YuriNachos wants to merge 1 commit into
yorukot:mainfrom
YuriNachos:YuriNachos/w3-superfile
Open

fix(preview): skip thumbnail generation when image preview is disabled#1621
YuriNachos wants to merge 1 commit into
yorukot:mainfrom
YuriNachos:YuriNachos/w3-superfile

Conversation

@YuriNachos

@YuriNachos YuriNachos commented Aug 8, 2026

Copy link
Copy Markdown

Description

When show_image_preview = false, hovering a video/PDF/PS file in the preview panel still
spawned the thumbnail-generation subprocess (ffmpeg / pdftoppm / gs) only to discard the
result — renderImagePreview's disabled guard threw the generated thumbnail away, but only
after the child process had already run. This wastes CPU and, as reported in #1620, surfaces
as visible ffmpeg child processes while image preview is turned off.

Model.RenderWithPath now checks common.Config.ShowImagePreview before
GetThumbnailOrGenerate and returns the existing Image preview is disabled placeholder, so no
thumbnail subprocess is spawned while image preview is disabled.

Fixes #1620

Changes

  • src/internal/ui/preview/render.go — add an early-return guard at the top of the
    thumbnail-eligible branch (if m.thumbnailGenerator != nil && m.thumbnailGenerator.SupportsExt(ext))
    that returns FilePreviewImagePreviewDisabledText when ShowImagePreview is false, before
    GetThumbnailOrGenerate is reached. The enabled path is unchanged; the existing guard inside
    renderImagePreview is kept as defense-in-depth. The guard is scoped to the thumbnail branch so
    that text and directory preview are unaffected.

Test plan

  • make dev — green (golangci-lint v2.12.2 0 issues, all tests pass, spf builds)
  • gofmt -l . — empty
  • go test ./src/internal/ui/preview/ -run TestRenderWithPathDoesNotGenerateThumbnailWhenImagePreviewDisabled -v — PASS (and FAILs on unmodified main, where it captures the Error generating thumbnail / ffmpeg exit status log)
  • Manual reproducer from the issue: set show_image_preview = false, hover a video file in the preview panel — no ffmpeg child process is spawned (verified via the captured slog output; runtime drops to ~0)

🤖 Written with Cursor

Co-authored-by: Cursor

Summary by CodeRabbit

  • Bug Fixes
    • Disabled image previews now avoid unnecessary thumbnail generation.
    • Video files correctly display the disabled-preview message without launching background thumbnail processes.
  • Tests
    • Added regression coverage to verify thumbnail generation is skipped when image previews are turned off.

When show_image_preview = false, Model.RenderWithPath still spawned the
thumbnail-generation subprocess (ffmpeg/pdftoppm/gs) for video/PDF/PS
files, only to discard the result in renderImagePreview's disabled guard.
Add an early ShowImagePreview check inside the thumbnail-eligible branch,
before GetThumbnailOrGenerate, returning the existing disabled placeholder
so no subprocess runs while image preview is disabled. The visible output
is unchanged; only the wasted child process is eliminated.

Fixes yorukot#1620

🤖 Written with Cursor

Co-authored-by: Cursor
@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown

🎉 Thank you for your first contribution to superfile!

We’re really excited to have you here 🙌

A maintainer might ask you to make a few changes before we can merge this PR.
That’s totally normal and part of the process. Don’t worry, we’ll help guide you through it.

👉 Please also take a moment to review our Contribution Guide

If you have any questions, feel free to open a Discussion or just ask in the comments!

@github-actions github-actions Bot added test related PR / Issue related to testcases or testing in general. awaiting pr review labels Aug 8, 2026
@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6c49ed53-c854-4406-8961-49cacbb9dcff

📥 Commits

Reviewing files that changed from the base of the PR and between a97381e and d1e7d49.

📒 Files selected for processing (2)
  • src/internal/ui/preview/render.go
  • src/internal/ui/preview/render_test.go

📝 Walkthrough

Walkthrough

The preview renderer now checks ShowImagePreview before generating thumbnails. A regression test verifies that disabled previews prevent video thumbnail generation while preserving the render path.

Changes

Preview generation guard

Layer / File(s) Summary
Guard thumbnail generation and regression test
src/internal/ui/preview/render.go, src/internal/ui/preview/render_test.go
RenderWithPath returns the disabled-preview message before thumbnail generation. The regression test verifies that rendering a video does not emit thumbnail-generation logs when previews are disabled.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • yorukot/superfile#1506: Both changes modify preview rendering, although that PR adds scroll-state rendering logic.
  • yorukot/superfile#1619: This PR also concerns video thumbnail generation in the preview path, but changes a different behavior layer.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: skipping thumbnail generation when image previews are disabled.
Linked Issues check ✅ Passed The change adds an early preview-disabled guard before thumbnail generation, which satisfies issue #1620.
Out of Scope Changes check ✅ Passed The code and regression test directly support the preview-rendering fix and contain no unrelated changes.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@YuriNachos

Copy link
Copy Markdown
Author

Following up on this since the fix is small and self-contained — RenderWithPath now checks show_image_preview before any thumbnail subprocess is spawned, so hovering a video with previews off no longer wakes up ffmpeg (the reproducer from #1620). Is there anything you'd like adjusted before it lands, or anything blocking the review on your end? The windows-latest failure is the same flake that's already red on main, so I've left the branch as-is. Glad to rebase, extend the test, or otherwise iterate if that helps it move.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting pr review test related PR / Issue related to testcases or testing in general.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

File preview / thumbnail creation subprocesses still run if image_preview config set to false

1 participant