-
Notifications
You must be signed in to change notification settings - Fork 1.5k
zed@0.210.4: Add context menu #16523
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
WalkthroughAdds Windows Registry install/uninstall scripts that create/remove "Open with Zed" context menu entries for files, directories, and directory backgrounds, and extends the installer manifest with Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Explorer
participant Registry as Windows Registry
participant ZedApp as Zed.exe
rect rgb(240,248,255)
User->>Explorer: Right-click file/dir/background
Explorer->>Registry: Read context menu handler for clicked item
Registry-->>Explorer: Return "Open with Zed" command (with %1/%V)
Explorer->>ZedApp: Launch "Zed.exe" with argument (%1 or %V)
ZedApp-->>User: Open item in editor
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/zed/uninstall-context.reg (1)
3-8: Consider simplifying by removing redundant subkey deletions.When deleting a registry key, all its subkeys are automatically deleted. Lines 4, 6, and 8 (the
\commandsubkeys) are redundant since they're already deleted when their parent keys (lines 3, 5, 7) are removed.Apply this diff to simplify:
-[-HKEY_CURRENT_USER\Software\Classes\*\shell\Open with &Zed] -[-HKEY_CURRENT_USER\Software\Classes\*\shell\Open with &Zed\command] -[-HKEY_CURRENT_USER\Software\Classes\Directory\shell\Open with &Zed] -[-HKEY_CURRENT_USER\Software\Classes\Directory\shell\Open with &Zed\command] -[-HKEY_CURRENT_USER\Software\Classes\Directory\Background\shell\Open with &Zed] -[-HKEY_CURRENT_USER\Software\Classes\Directory\Background\shell\Open with &Zed\command] +[-HKEY_CURRENT_USER\Software\Classes\*\shell\Open with &Zed] +[-HKEY_CURRENT_USER\Software\Classes\Directory\shell\Open with &Zed] +[-HKEY_CURRENT_USER\Software\Classes\Directory\Background\shell\Open with &Zed]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
scripts/zed/install-context.reg(1 hunks)scripts/zed/uninstall-context.reg(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: qyurila
Repo: ScoopInstaller/Extras PR: 16346
File: bucket/zed.json:18-21
Timestamp: 2025-10-16T06:32:15.420Z
Learning: The Zed Windows application has three separate executables after InnoSetup extraction:
- `zed.exe` at root: GUI application (used in shortcuts)
- `bin/zed`: Shell script for WSL integration
- `bin/zed.exe`: Zed CLI binary with different behavior than the GUI
All three should be preserved in the manifest.
📚 Learning: 2025-10-16T06:32:15.420Z
Learnt from: qyurila
Repo: ScoopInstaller/Extras PR: 16346
File: bucket/zed.json:18-21
Timestamp: 2025-10-16T06:32:15.420Z
Learning: The Zed Windows application has three separate executables after InnoSetup extraction:
- `zed.exe` at root: GUI application (used in shortcuts)
- `bin/zed`: Shell script for WSL integration
- `bin/zed.exe`: Zed CLI binary with different behavior than the GUI
All three should be preserved in the manifest.
Applied to files:
scripts/zed/install-context.regscripts/zed/uninstall-context.reg
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: WindowsPowerShell
🔇 Additional comments (2)
scripts/zed/install-context.reg (2)
5-5: Verify the correct case for the executable name.The registry entries reference
Zed.exe(capital Z), but based on learnings, the GUI application iszed.exe(lowercase). While Windows file paths are case-insensitive, using the correct case is best practice.Based on learnings: The Zed GUI application is located at
zed.exe(lowercase) at the root of the installation directory.Please confirm the correct executable name case and update accordingly.
Also applies to: 7-7, 11-11, 13-13, 17-17, 19-19
3-4: Structure and registry key organization look good.The registry structure correctly:
- Uses
HKEY_CURRENT_USERfor user-level registration- Sets up context menu entries for files (
*\shell), directories (Directory\shell), and directory backgrounds (Directory\Background\shell)- Uses
&Zedfor keyboard accelerator (Alt+Z)- Defines display names, icons, and commands appropriately
- Uses
%1for files and%Vfor directories (correct Explorer arguments)This matches the pattern used by VS Code and other editor integrations.
Also applies to: 6-6, 9-10, 12-12, 15-16, 18-18
scripts/zed/install-context.reg
Outdated
|
|
||
| [HKEY_CURRENT_USER\Software\Classes\*\shell\Open with &Zed] | ||
| @="Open with &Zed" | ||
| "Icon"="C:\\Users\\mcourbet\\scoop\\apps\\zed\\current\\Zed.exe" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Critical: Hardcoded user-specific paths will fail for other users.
All registry entries contain the hardcoded path C:\\Users\\mcourbet\\scoop\\apps\\zed\\current\\Zed.exe, which:
- Won't work for users with different usernames
- Won't work if Scoop is installed in a non-default location
- Makes this registry script unusable in its current form
For Scoop manifests, registry scripts typically need to be templated or generated dynamically during installation to substitute the actual installation path.
Please verify how Scoop handles registry scripts with dynamic paths. You may need to:
- Use Scoop's template substitution if supported for
.regfiles - Generate the registry entries programmatically during installation
- Use a PowerShell pre-install script to create the registry file with the correct path
Run this script to check how other Scoop packages handle context menu registration:
Also applies to: 7-7, 11-11, 13-13, 17-17, 19-19
🏁 Script executed:
#!/bin/bash
# Search for other packages with context menu registry scripts
fd -e reg -e ps1 . --exec rg -l "shell\\" {} \; | head -20Length of output: 8879
🏁 Script executed:
#!/bin/bash
# Search for .reg files in the repository to understand patterns
find . -name "*.reg" -type f 2>/dev/null | head -20
# Also search for other examples of context menu scripts
find . -name "*context*" -type f 2>/dev/null | head -20Length of output: 1617
🏁 Script executed:
#!/bin/bash
# Check if there's a post_install or pre_install script that might handle registry
fd -e ps1 scripts/zed/ --exec cat {} \; 2>/dev/nullLength of output: 0
Then I'll search for information about Scoop's registry handling patterns.
</web_search>
🏁 Script executed:
#!/bin/bash
# Compare with another package's context.reg file
echo "=== Alacritty install-context.reg ==="
cat ./scripts/alacritty/install-context.reg
echo ""
echo "=== Fork install-context.reg ==="
cat ./scripts/fork/install-context.regLength of output: 1156
🏁 Script executed:
#!/bin/bash
# Check harmonoid's PowerShell approach for dynamic paths
echo "=== Harmonoid install script ==="
cat ./scripts/harmonoid/install-harmonoid-context.ps1Length of output: 493
🌐 Web query:
Scoop package manager registry script dynamic paths templating .reg files
💡 Result:
Short answer — do it in the manifest using Scoop variables plus a post_install PowerShell step that templates the .reg and imports it.
Why: Scoop manifest JSON supports install-dir variables ($dir, $persist, $env:SCOOP) in env_set/etc, but .reg files must be written/edited at install time and imported with reg.exe or reg import in a post_install script. See Scoop docs on manifest variables and env handling. [1][2][3]
Minimal example (manifest snippet):
- Use a post_install PowerShell command to replace placeholders with $dir or $env:SCOOP and then import:
{
"version": "1.0",
"url": "https://example.com/app.zip",
"extract_dir": "app",
"bin": "app.exe",
"post_install": [
"powershell -NoProfile -ExecutionPolicy Bypass -Command "(Get-Content '$dir\templates\app-context.reg') -replace '\$\{DIR\}', [regex]::Escape('$dir') | Set-Content -Path "$env:TEMP\app-context.reg"; reg import \"$env:TEMP\app-context.reg\"""
]
}
Notes:
- Use $dir inside the manifest (Scoop expands it when running the command). [1][2]
- Prefer writing the .reg as a template with a unique placeholder (e.g. ${DIR}) and replace it in post_install to avoid escaping issues.
- You can also use PowerShell to directly write registry keys (New-ItemProperty/Set-ItemProperty) to avoid .reg files entirely. [2]
- If you need to reference the Scoop root, use $env:SCOOP or detect $env:USERPROFILE\scoop. [1][3]
Sources:
[1] Scoop concepts / autoupdate & variables. (scoop.netlify.app)
[2] Scoop App Manifests (env_set, post_install behavior). (github.com)
[3] Examples of using reg import after scoop install (community examples). (qiita.com)
If you want, I can produce a ready-to-paste full manifest and a robust PowerShell templating snippet that handles quoting/64-bit registry view.
Citations:
- 1: https://scoop.netlify.app/concepts/?utm_source=openai
- 2: https://github.com/ScoopInstaller/Scoop/wiki/App-Manifests?utm_source=openai
- 3: https://qiita.com/rhene/items/d8a0c0c7d637904e14da?utm_source=openai
🏁 Script executed:
#!/bin/bash
# Check zed.json manifest
echo "=== Zed manifest (zed.json) ==="
cat ./bucket/zed.json
echo ""
echo "=== Zed uninstall-context.reg ==="
cat ./scripts/zed/uninstall-context.regLength of output: 1640
Critical: Hardcoded user-specific paths and incorrect executable case prevent functionality.
The install-context.reg file has two critical issues:
-
Hardcoded user path (lines 5, 7, 11, 13, 17, 19):
C:\\Users\\mcourbet\\scoop\\apps\\zed\\current\\Zed.exewill only work for this developer and will fail for all other users. This is inconsistent with the repository pattern—other packages likealacrittyandforkuse template variables ($alacrittyPath,$fork-path). -
Wrong executable case: The registry references
Zed.exe(capital Z), but the manifest specifieszed.exe(lowercase) in both thebinandshortcutsfields. Use the lowercase variant consistently.
Required fixes:
- Replace hardcoded path with a template variable (e.g.,
$zedPath) - Change
Zed.exetozed.exethroughout - Add a
post_installPowerShell script tozed.jsonto template the.regfile with the actual installation path (using$dir) and import it, similar to howforkandalacrittyhandle dynamic paths in this repository
🤖 Prompt for AI Agents
In scripts/zed/install-context.reg around line 5, the registry file uses a
hardcoded user-specific path and the wrong executable case; replace every
hardcoded "C:\\Users\\mcourbet\\scoop\\apps\\zed\\current\\Zed.exe" with a
template variable like "$zedPath" and change "Zed.exe" to "zed.exe" everywhere;
then add a post_install PowerShell step in zed.json that templates the .reg file
with the actual installation directory (use $dir or the repo’s templating
pattern shown in fork/alacritty), writes the rendered .reg to disk and imports
it via regedit to ensure correct, per-user installation.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
bucket/zed.json (1)
6-9: Clarify instructions: notes mention manualreg importwhenpost_installautomates it.The notes instruct users to manually run
reg import, but thepost_installscript already creates and processes the registry file automatically. Either:
- Remove the notes if the script handles setup fully, or
- Update notes to explain that the context menu is added automatically on install, or clarify when manual intervention is needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
bucket/zed.json(2 hunks)scripts/zed/install-context.reg(1 hunks)scripts/zed/uninstall-context.reg(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- scripts/zed/uninstall-context.reg
- scripts/zed/install-context.reg
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: qyurila
Repo: ScoopInstaller/Extras PR: 16346
File: bucket/zed.json:18-21
Timestamp: 2025-10-16T06:32:15.420Z
Learning: The Zed Windows application has three separate executables after InnoSetup extraction:
- `zed.exe` at root: GUI application (used in shortcuts)
- `bin/zed`: Shell script for WSL integration
- `bin/zed.exe`: Zed CLI binary with different behavior than the GUI
All three should be preserved in the manifest.
📚 Learning: 2025-10-16T06:32:15.420Z
Learnt from: qyurila
Repo: ScoopInstaller/Extras PR: 16346
File: bucket/zed.json:18-21
Timestamp: 2025-10-16T06:32:15.420Z
Learning: The Zed Windows application has three separate executables after InnoSetup extraction:
- `zed.exe` at root: GUI application (used in shortcuts)
- `bin/zed`: Shell script for WSL integration
- `bin/zed.exe`: Zed CLI binary with different behavior than the GUI
All three should be preserved in the manifest.
Applied to files:
bucket/zed.json
🔇 Additional comments (2)
bucket/zed.json (2)
36-36: Verify: Registry file for uninstall exists and is correctly formatted.The
pre_uninstallscript importsuninstall-context.regwithout validation. Ensure the file exists and contains valid registry entries to remove the context menu properly.
22-34: Remove$globalcheck — variable is not provided by Scoop.Scoop does not expose a
$globalvariable to post_install scripts. The condition on line 28 will always be falsy, causing machine-wide installs to always useHKEY_CURRENT_USERinstead ofHKEY_LOCAL_MACHINE.To detect global installs, check the install directory or environment variable:
- Test if
$diris under the global path:if ($dir -like 'C:\ProgramData\scoop*')- Or check the
SCOOP_GLOBALenvironment variable:$env:SCOOP_GLOBAL⛔ Skipped due to learnings
Learnt from: o-l-a-v Repo: ScoopInstaller/Extras PR: 16328 File: bucket/winutil.json:0-0 Timestamp: 2025-10-13T09:37:06.093Z Learning: When reviewing code in Scoop manifests, always verify the actual file content if the diff appears incomplete or unusual, as diff context can be truncated or displayed incorrectly. The pattern `[string[]](...).ForEach{...}` is commonly used in post_uninstall scripts across the Extras repository.Learnt from: qyurila Repo: ScoopInstaller/Extras PR: 16346 File: bucket/zed.json:18-21 Timestamp: 2025-10-16T06:32:15.420Z Learning: The Zed Windows application has three separate executables after InnoSetup extraction: - `zed.exe` at root: GUI application (used in shortcuts) - `bin/zed`: Shell script for WSL integration - `bin/zed.exe`: Zed CLI binary with different behavior than the GUI All three should be preserved in the manifest.Learnt from: SorYoshino Repo: ScoopInstaller/Extras PR: 16333 File: bucket/potplayer.json:55-64 Timestamp: 2025-10-14T11:24:17.524Z Learning: In PotPlayer manifests (bucket/potplayer.json), the Capture and Playlist folders are created at runtime (after running the application), not included in the installer, so they should only be in the persist list and not in post_install scripts.
| "post_install": [ | ||
| "$exepath = \"$dir\\zed.exe\".Replace('\\', '\\\\')", | ||
| "'install-context.reg', 'uninstall-context.reg' | ForEach-Object {", | ||
| " if (Test-Path \"$bucketsdir\\$bucket\\scripts\\$app\\$_\") {", | ||
| " $content = Get-Content \"$bucketsdir\\$bucket\\scripts\\$app\\$_\"", | ||
| " $content = $content.Replace('$zed-path', $exepath)", | ||
| " if ($global) {", | ||
| " $content = $content.Replace('HKEY_CURRENT_USER', 'HKEY_LOCAL_MACHINE')", | ||
| " }", | ||
| " }", | ||
| " $content | Set-Content -Path \"$dir\\$_\" -Encoding ascii", | ||
| "}" | ||
| ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: PowerShell logic in post_install will fail if registry files are missing.
The $content variable is only initialized inside the if block, but the Set-Content call at line 32 executes regardless. If Test-Path returns false, $content is undefined, causing the script to fail or write stale data from a previous iteration.
Restructure the script so Set-Content only executes when the file is found:
"post_install": [
"$exepath = \"$dir\\zed.exe\".Replace('\\', '\\\\')",
"'install-context.reg', 'uninstall-context.reg' | ForEach-Object {",
- " if (Test-Path \"$bucketsdir\\$bucket\\scripts\\$app\\$_\") {",
+ " $scriptPath = \"$bucketsdir\\$bucket\\scripts\\$app\\$_\"",
+ " if (Test-Path $scriptPath) {",
" $content = Get-Content \"$bucketsdir\\$bucket\\scripts\\$app\\$_\"",
" $content = $content.Replace('$zed-path', $exepath)",
" if ($global) {",
" $content = $content.Replace('HKEY_CURRENT_USER', 'HKEY_LOCAL_MACHINE')",
" }",
+ " $content | Set-Content -Path \"$dir\\$_\" -Encoding ascii",
" }",
- " $content | Set-Content -Path \"$dir\\$_\" -Encoding ascii",
"}"
],🤖 Prompt for AI Agents
In bucket/zed.json around lines 22 to 34, the post_install PowerShell loop calls
Set-Content unconditionally even when Test-Path is false, which leaves $content
undefined; modify the script so Set-Content is executed only inside the
Test-Path true branch (i.e., move the Set-Content call into the if block) and
ensure $content is only referenced after being initialized; also preserve the
ascii encoding and the existing replacements for $zed-path and optional
HKEY_LOCAL_MACHINE substitution inside that conditional.
Closes #16522
<manifest-name[@version]|chore>: <general summary of the pull request>Summary by CodeRabbit