Skip to content

Conversation

@Vixb1122
Copy link
Contributor

@Vixb1122 Vixb1122 commented Oct 31, 2025

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

Summary by CodeRabbit

  • Chores
    • Installer now disables automatic updates by default in the app's settings.
    • Existing user settings are preserved (including comments) while ensuring the update preference is set to off.

@coderabbitai
Copy link

coderabbitai bot commented Oct 31, 2025

Walkthrough

Adds a post-install step to the Zed Scoop bucket that ensures Zed's settings.json exists under APPDATA and sets auto_update to false, preserving existing settings and comments, and writing the updated JSON back with UTF-8 encoding.

Changes

Cohort / File(s) Change Summary
Zed installer configuration
bucket/zed.json
Adds a post-install block that creates/verifies the Zed settings directory, reads existing settings.json (preserving comments), sets or appends auto_update: false, and writes the file with UTF-8 encoding after archive extraction.

Sequence Diagram(s)

sequenceDiagram
    participant Installer
    participant Filesystem
    participant Network
    Note over Installer,Filesystem: Post-install: disable Zed auto-updates
    Installer->>Filesystem: Ensure %APPDATA%/Zed exists
    alt settings.json exists
        Installer->>Filesystem: Read settings.json (keep comments)
    else settings.json missing
        Installer->>Network: Download initial_user_settings.json
        Network-->>Installer: initial_user_settings.json
    end
    Installer->>Installer: Ensure auto_update = false (preserve other keys)
    Installer->>Filesystem: Write updated settings.json (UTF-8)
    Note right of Filesystem: Installation complete
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Check JSON handling (comment preservation and UTF-8 write).
  • Verify APPDATA path resolution and directory creation on Windows.
  • Confirm network fallback and error handling for downloading initial settings.

Possibly related PRs

Suggested reviewers

  • z-Fng
  • chawyehsu

Poem

🐇 In APPDATA where tiny settings hide,
I hop and nudge the JSON side by side.
auto_update off, no sudden surprise,
A gentle hush beneath Zed's skies. ✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The PR description is minimal and largely incomplete compared to the template requirements. While the author has checked both required checkboxes (confirming use of conventional PR title and having read the Contributing Guide), the description lacks substantive content including: a summary of the changes in the PR body, an issue reference (Closes #XXXX or Relates to #XXXX as shown in the template), and any explanation of the modification. The description consists solely of two completed checkboxes with no additional context or details about the change.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title "zed: Update installer script to turn off auto update" directly and accurately summarizes the main change described in the raw_summary, which adds logic to disable Zed auto updates in the installer script. The title uses the conventional format specified in the template (manifest-name: description), is specific about both what is being changed (the installer script) and why (to turn off auto update), and clearly conveys the primary purpose of the changeset to someone reviewing the project history.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3ac70b0 and 370f27d.

📒 Files selected for processing (1)
  • bucket/zed.json (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • bucket/zed.json
⏰ 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). (2)
  • GitHub Check: WindowsPowerShell
  • GitHub Check: PowerShell

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.

@github-actions
Copy link
Contributor

All changes look good.

Wait for review from human collaborators.

zed

  • Lint
  • Description
  • License
  • Hashes
  • Checkver
  • Autoupdate

Check the full log for details.

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

🧹 Nitpick comments (1)
bucket/zed.json (1)

16-30: Add error handling for JSON parsing and file operations.

The auto-update disable logic lacks error handling. If the JSON file is corrupted or parsing fails, ConvertFrom-Json will throw an unhandled exception, causing the installation to fail. Similarly, file system operations (New-Item, Out-File) may fail silently or raise exceptions if permissions are insufficient. Wrap the JSON operations in a try-catch block to gracefully handle errors and provide helpful diagnostics.

Additionally, the condition on line 27 (-ne $false) avoids unnecessary writes but could be clearer with an inline comment explaining the intent (e.g., "only write if we modified the setting").

Consider this improved structure:

  "# Turn off Auto Update",
  "$zedSettingsPath = Join-Path $env:APPDATA \"Zed\\settings.json\"",
  "$zedDir = Split-Path $zedSettingsPath",
  "if (!(Test-Path $zedDir)) {",
  "    New-Item -ItemType Directory -Path $zedDir | Out-Null",
  "}",
- "if (Test-Path $zedSettingsPath) {",
-     "$jsonContent = Get-Content $zedSettingsPath -Raw | ConvertFrom-Json",
- "} else {",
+ "try {",
+     "if (Test-Path $zedSettingsPath) {",
+         "$jsonContent = Get-Content $zedSettingsPath -Raw | ConvertFrom-Json",
+     "} else {",
-     "$jsonContent = @{}",
- "}",
- "if ($jsonContent.\"auto_update\" -ne $false) {",
+         "$jsonContent = @{}",
+     "}",
+ "} catch {",
+     "Write-Host 'Warning: Failed to parse Zed settings. Auto-update disable skipped.' -ForegroundColor Yellow",
+     "exit 0",
+ "}",
+ "# Only write settings if auto_update is not already false",
+ "if ($jsonContent.\"auto_update\" -ne $false) {",
      "$jsonContent | Add-Member -NotePropertyName \"auto_update\" -NotePropertyValue $false -Force",
      "$jsonContent | ConvertTo-Json -Depth 10 | Out-File -FilePath $zedSettingsPath -Encoding UTF8",
- "}"
+ "}",
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5a8d5ab and 3ac70b0.

📒 Files selected for processing (1)
  • bucket/zed.json (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 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
⏰ 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). (2)
  • GitHub Check: WindowsPowerShell
  • GitHub Check: PullRequestHandler
🔇 Additional comments (1)
bucket/zed.json (1)

14-15: Verify the extraction logic and -Removal flag necessity.

Line 14 extracts to {app}, and the new line 15 extracts the same archive to {code_GetInstallDir} with the -Removal flag. Extracting the same file twice seems unusual—clarify whether this is intentional (e.g., for backward compatibility or extracting different components to different locations). Also confirm that -Removal deletes the archive after the first extraction and doesn't prevent the second extraction.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant