feat: add Opera and Opera GX browser cache support#4
Conversation
Vortrix5
left a comment
There was a problem hiding this comment.
Thanks for this, and for closing #2! The idea is exactly right and the code is clean and matches the surrounding style. I think the Opera path details need a second look before this can land, because as written I suspect it scans nothing on a real Opera install. I don't have Opera on my test machine, so I've framed the path points as "please verify" rather than hard facts. Two correctness items and one test item:
1. Cache is probably under Local, not Roaming (please verify)
You're correct that Opera keeps its profile under %APPDATA%\Opera Software\Opera Stable (roaming). But Opera relocates the on-disk cache to %LOCALAPPDATA%, i.e.
%LOCALAPPDATA%\Opera Software\Opera Stable\Cache
%LOCALAPPDATA%\Opera Software\Opera Stable\Code Cache
%LOCALAPPDATA%\Opera Software\Opera Stable\GPUCache
(roaming a multi-GB cache would be slow, so Chromium browsers keep prefs in roaming but cache in local). If that's right, scanning %APPDATA% finds the profile folder but no Cache subdir, so the category stays empty. Could you confirm on a real Opera + Opera GX install where the Cache folder actually sits, and point the scan there? sifty junk scan (dry-run, safe) will show whether any Opera roots are picked up.
2. Opera doesn't use Chrome-style profile subfolders
The block reuses the Chromium profile filter:
if p.is_dir() and (p.name == "Default" or p.name.startswith("Profile"))Opera is different: the Opera Stable folder is the default profile, so Cache / Code Cache / GPUCache sit directly under it, not under a Default subfolder. With this filter the loop matches nothing for a standard single-profile Opera. So even with the base dir fixed, the profile-nesting logic needs to differ from Chrome (likely: treat the Opera Stable dir itself as the profile root).
3. Please add a test
Repo rule (CONTRIBUTING.md + the PR template): new core logic needs a matching test. The cleanest spot is to extend test_browser_cache_covers_all_profiles_and_firefox in tests/test_junk.py, or add an Opera-specific one mirroring the Chrome fixture. Set up a fake Opera cache tree under whichever base dir turns out to be correct, monkeypatch the env var, and assert the cache roots are picked up. That test is also the fastest way to confirm items 1 and 2, since it will fail if the paths are off.
Minor (non-blocking)
- Once the base dir and profile layout are settled, the Opera block is nearly identical to the Chromium loop. Worth folding the two into a shared helper (base dir + profile-detection strategy as a small parameter) rather than duplicating, but only after it's correct.
- Tiny PR-description typo: a couple of bullets start with
- -. - Title nit: Sifty's convention is a parenthesised tag,
(feat) add Opera..., notfeat: .... We just added a check that enforces this on PR titles.
None of this is a safety concern, the change only adds cache roots and goes nowhere near deletion. Happy to help if the Local-vs-Roaming question is fiddly to pin down. Thanks again for jumping on this one.
What does this PR do?
Adds Opera and Opera GX to the browser-cache junk category. Closes #2
Opera stores its User Data under
%APPDATA%\Opera Software\Opera StableandOpera GX Stable(roaming appdata, not localappdata), with the same Cache/Code Cache/GPUCache layout as other Chromium browsers.Changes:
_roaming_appdata()helper to look up%APPDATA%Safety checklist
Sifty deletes files, so every PR keeps these promises:
pytestis green, includingtests/test_safety.pyos.remove,shutil.rmtree,Path.unlink); everything goes throughsafety.trash()Notes for the reviewer