seelen-ui: Add file associations and URL protocol handler#17728
Conversation
📝 WalkthroughWalkthroughThe pull request modifies the Seelen-UI manifest to support file association and URL handler registration. The manifest updates the Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bucket/seelen-ui.json (1)
9-35:⚠️ Potential issue | 🟠 Major | ⚡ Quick winImport the generated
.regfiles during install.Lines 29-35 only materialize the registry payloads into
$dir; nothing here actually registers them. That means a freshscoop install seelen-uistill leaves the protocol/file association inactive until the user runs the notes manually, so the browser-launch flow from issue#17722is still broken out of the box and upgrades will not refresh existing registrations either.Suggested change
"post_install": [ "$seelen_dir = $dir -replace '\\\\', '\\\\'", "Get-ChildItem -Path \"$bucketsdir\\$bucket\\scripts\\$app\" -Filter '*.reg' -File | ForEach-Object {", " $content = Get-Content -Path $_.FullName -Encoding utf8", " if ($global) { $content = $content -replace 'HKEY_CURRENT_USER', 'HKEY_LOCAL_MACHINE' }", " $content -replace '{{seelen_dir}}', $seelen_dir | Set-Content -Path \"$dir\\$($_.Name)\" -Encoding unicode", - "}" + "}", + "Get-ChildItem -Path $dir -Filter '*.reg' -File | Where-Object Name -notlike 'un*.reg' | ForEach-Object {", + " Start-Process -FilePath 'reg.exe' -ArgumentList @('import', $_.FullName) -WindowStyle Hidden -Wait", + "}" ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bucket/seelen-ui.json` around lines 9 - 35, The post_install currently writes the .reg files to $dir but never imports them; update the post_install block (after the Get-ChildItem ... | ForEach-Object ... Set-Content sequence) to iterate the same '*.reg' files and invoke reg import (or Start-Process 'reg.exe' /c) for each generated file so the registry entries are actually applied during install/upgrade; use the same $seelen_dir/$dir/$bucket variables and preserve the $global conditional logic, and add minimal error checking/logging so failures don't break install but are reported.
🧹 Nitpick comments (1)
bucket/seelen-ui.json (1)
1-64: Please run the Scoop manifest checks before merge.For this manifest, I’d verify both architectures and the full registry flow locally:
scoop config debug true scoop config gh_token <your-read-only-github-token> # optional, GitHub releases only .\bin\checkver.ps1 -App seelen-ui -f .\bin\formatjson.ps1 -App seelen-ui scoop install .\bucket\seelen-ui.json -a 64bit scoop install .\bucket\seelen-ui.json -a arm64After that, validate that the generated
.regfiles import cleanly for both user and--globalinstalls, and that uninstall removes the.sluandseelen-ui.uriregistrations.As per coding guidelines, "Provide clear instructions for testing the manifest locally before submission" for Scoop manifests.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@bucket/seelen-ui.json` around lines 1 - 64, The manifest lacks a local-test instruction and explicit verification steps in the file metadata; add a concise "testing" note to the manifest (e.g., under a new "notes" entry or expand existing "notes") that instructs reviewers to run the Scoop manifest checks and install both architectures and global vs user registry flows; reference the manifest symbols seelen-ui.json, the pre_install/post_install scripts and uninstaller script so testers know to run "checkver/formatjson" and to test "scoop install .\\bucket\\seelen-ui.json -a 64bit" and "-a arm64" as well as verifying the generated .reg imports for user and --global installs and that uninstall cleans up .slu and seelen-ui.uri registrations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@bucket/seelen-ui.json`:
- Line 28: The pre_install cleanup pattern currently only removes items matching
'uninst*' leaving files like 'unregister-url-handler.reg' behind; update the
"pre_install" entry in seelen-ui.json to use a broader pattern (e.g., 'un*' or
include 'un*.reg') so the ForEach-Object removal covers generated uninstall
payloads such as 'unregister-url-handler.reg' in $dir; locate the "pre_install"
key and replace 'uninst*' with the broader pattern to ensure stale uninstall
files are removed before install.
---
Outside diff comments:
In `@bucket/seelen-ui.json`:
- Around line 9-35: The post_install currently writes the .reg files to $dir but
never imports them; update the post_install block (after the Get-ChildItem ... |
ForEach-Object ... Set-Content sequence) to iterate the same '*.reg' files and
invoke reg import (or Start-Process 'reg.exe' /c) for each generated file so the
registry entries are actually applied during install/upgrade; use the same
$seelen_dir/$dir/$bucket variables and preserve the $global conditional logic,
and add minimal error checking/logging so failures don't break install but are
reported.
---
Nitpick comments:
In `@bucket/seelen-ui.json`:
- Around line 1-64: The manifest lacks a local-test instruction and explicit
verification steps in the file metadata; add a concise "testing" note to the
manifest (e.g., under a new "notes" entry or expand existing "notes") that
instructs reviewers to run the Scoop manifest checks and install both
architectures and global vs user registry flows; reference the manifest symbols
seelen-ui.json, the pre_install/post_install scripts and uninstaller script so
testers know to run "checkver/formatjson" and to test "scoop install
.\\bucket\\seelen-ui.json -a 64bit" and "-a arm64" as well as verifying the
generated .reg imports for user and --global installs and that uninstall cleans
up .slu and seelen-ui.uri registrations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c4945be9-6cea-47a5-818f-17f173a716cd
📒 Files selected for processing (5)
bucket/seelen-ui.jsonscripts/seelen-ui/install-associations.regscripts/seelen-ui/register-url-handler.regscripts/seelen-ui/uninstall-associations.regscripts/seelen-ui/unregister-url-handler.reg
|
/verify |
|
All changes look good. Wait for review from human collaborators. seelen-ui
|
|
Sorry, I forgot to send |
|
No worries. |
|
Thanks <3 |
Summary
Enhances
seelen-uimanifest by adding automatic registry scripts for file associations (.slu) and URL protocol handling (seelen-ui.uri), while also improving license metadata.Related issues or pull requests
Changes
.regfiles for system integration..regfiles from templates in thescriptsdirectory.{{seelen_dir}}.HKEY_CURRENT_USERandHKEY_LOCAL_MACHINEbased on the--globalflag.Testing
The test results are as follows:
<manifest-name[@version]|chore>: <general summary of the pull request>