Skip to content

Conversation

@nagromc
Copy link
Contributor

@nagromc nagromc commented Nov 3, 2025

Closes #16522

  • Use conventional PR title: <manifest-name[@version]|chore>: <general summary of the pull request>
  • I have read the Contributing Guide

Summary by CodeRabbit

  • New Features
    • Adds Windows context menu integration so users can right-click files, folders, or folder backgrounds to open them with Zed.
    • Includes installation and uninstallation scripts to manage the context menu.
    • Installer manifest enhanced with install/uninstall hooks and user-facing notes to support post-install and pre-uninstall actions.

@coderabbitai
Copy link

coderabbitai bot commented Nov 3, 2025

Walkthrough

Adds 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 notes, post_install, and pre_uninstall fields.

Changes

Cohort / File(s) Summary
Windows context menu scripts
scripts/zed/install-context.reg, scripts/zed/uninstall-context.reg
Adds an install .reg that registers "Open with Zed" handlers under HKEY_CURRENT_USER\Software\Classes for *, Directory, and Directory\Background (sets display name, icon, and command using %1/%V). Adds an uninstall .reg that deletes the three keys.
Installer manifest
bucket/zed.json
Adds notes (array), post_install (array), and pre_uninstall (string) fields to the manifest. No other behavioral 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
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Verify registry paths and key names under HKEY_CURRENT_USER\Software\Classes.
  • Confirm command-line argument placeholders (%1 for files, %V for directories/background) are correct for intended behavior.
  • Review bucket/zed.json additions for schema/validation compatibility and correct intended scripting semantics.
  • Ensure uninstall .reg removes exactly the keys created by install .reg.

Suggested labels

review-needed

Suggested reviewers

  • chawyehsu

Poem

🐰 Frolic in the shell, a click and Zed hops near,

Files and folders open wide — the editor appears,
A tiny rabbit cheers, "Context menu, you’re the key!" 🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title 'zed@0.210.4: Add context menu' clearly and directly summarizes the main change. It follows the conventional format specified in the template (<manifest-name[@version]>: <general summary>), identifying the package and version being updated along with the specific feature being added.
Description check ✅ Passed The pull request description is mostly complete and follows the required template structure. It includes the issue reference (Closes #16522) and has both required checkboxes marked as complete, indicating the author has used the conventional PR title format and read the contributing guide. However, the description could benefit from more detail about the specific changes being made.
Linked Issues check ✅ Passed The pull request successfully addresses the linked issue #16522 by adding Windows Explorer context menu support to Zed. The changes include install-context.reg registering the 'Open with Zed' context menu entries for files, directories, and directory backgrounds, and uninstall-context.reg removing these entries. The manifest updates in bucket/zed.json add post_install and pre_uninstall hooks to execute the registry scripts during installation and uninstallation.
Out of Scope Changes check ✅ Passed All changes in the pull request are directly related to implementing Windows Explorer context menu support as outlined in issue #16522. The three files modified (install-context.reg, uninstall-context.reg, and bucket/zed.json) are all necessary for adding and managing the context menu feature, with no extraneous modifications.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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 and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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 \command subkeys) 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

📥 Commits

Reviewing files that changed from the base of the PR and between 18bfa3f and 2d786ae.

📒 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.reg
  • scripts/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 is zed.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_USER for user-level registration
  • Sets up context menu entries for files (*\shell), directories (Directory\shell), and directory backgrounds (Directory\Background\shell)
  • Uses &Zed for keyboard accelerator (Alt+Z)
  • Defines display names, icons, and commands appropriately
  • Uses %1 for files and %V for 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


[HKEY_CURRENT_USER\Software\Classes\*\shell\Open with &Zed]
@="Open with &Zed"
"Icon"="C:\\Users\\mcourbet\\scoop\\apps\\zed\\current\\Zed.exe"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 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:

  1. Use Scoop's template substitution if supported for .reg files
  2. Generate the registry entries programmatically during installation
  3. 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 -20

Length 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 -20

Length 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/null

Length 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.reg

Length 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.ps1

Length 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:


🏁 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.reg

Length of output: 1640


Critical: Hardcoded user-specific paths and incorrect executable case prevent functionality.

The install-context.reg file has two critical issues:

  1. Hardcoded user path (lines 5, 7, 11, 13, 17, 19): C:\\Users\\mcourbet\\scoop\\apps\\zed\\current\\Zed.exe will only work for this developer and will fail for all other users. This is inconsistent with the repository pattern—other packages like alacritty and fork use template variables ($alacrittyPath, $fork-path).

  2. Wrong executable case: The registry references Zed.exe (capital Z), but the manifest specifies zed.exe (lowercase) in both the bin and shortcuts fields. Use the lowercase variant consistently.

Required fixes:

  1. Replace hardcoded path with a template variable (e.g., $zedPath)
  2. Change Zed.exe to zed.exe throughout
  3. Add a post_install PowerShell script to zed.json to template the .reg file with the actual installation path (using $dir) and import it, similar to how fork and alacritty handle 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.

@nagromc nagromc marked this pull request as draft November 3, 2025 12:17
@nagromc
Copy link
Contributor Author

nagromc commented Nov 3, 2025

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Nov 3, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link

@coderabbitai coderabbitai bot left a 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 manual reg import when post_install automates it.

The notes instruct users to manually run reg import, but the post_install script 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

📥 Commits

Reviewing files that changed from the base of the PR and between 2d786ae and 10a1ff6.

📒 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_uninstall script imports uninstall-context.reg without validation. Ensure the file exists and contains valid registry entries to remove the context menu properly.


22-34: Remove $global check — variable is not provided by Scoop.

Scoop does not expose a $global variable to post_install scripts. The condition on line 28 will always be falsy, causing machine-wide installs to always use HKEY_CURRENT_USER instead of HKEY_LOCAL_MACHINE.

To detect global installs, check the install directory or environment variable:

  • Test if $dir is under the global path: if ($dir -like 'C:\ProgramData\scoop*')
  • Or check the SCOOP_GLOBAL environment 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.

Comment on lines +22 to +34
"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",
"}"
],
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

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.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Zed: add Windows Explorer context menu

1 participant