Skip to content

Add cross-platform yt-dlp resolution without breaking Windows builds#3

Closed
Copilot wants to merge 3 commits intomainfrom
copilot/review-cross-functionality-pr
Closed

Add cross-platform yt-dlp resolution without breaking Windows builds#3
Copilot wants to merge 3 commits intomainfrom
copilot/review-cross-functionality-pr

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Feb 24, 2026

Reviewed PR #2 (cross-platform macOS/Linux support). The core refactoring is sound but contains a breaking change for Windows: extraResources.from was changed from "bin" to "electron/bin", which mismatches getBundledBinaryPath() resolving to <project_root>/bin/ — packaged Windows builds would fail to find yt-dlp.exe.

This PR applies the cross-platform changes safely:

  • Platform-dispatched ensureManagedYtDlpPath()winManagedYtDlpPath() preserves the original Windows logic verbatim; unixManagedYtDlpPath() uses which yt-dlp for Linux/macOS
  • Did not apply extraResources.from change — keeps "bin" to stay consistent with getBundledBinaryPath()'s dev-mode path (path.join(__dirname, "..", "bin", binaryName))
  • Fixed typo from PR Added compatibility for MacOS and Linux. #2: winManagedYtDplPathwinManagedYtDlpPath
  • README — added Linux/macOS build prerequisites with proper markdown (PR Added compatibility for MacOS and Linux. #2 had broken nested code blocks)
function ensureManagedYtDlpPath() {
  if (process.platform === "win32") {
    return winManagedYtDlpPath();     // unchanged original logic
  }
  if (process.platform === "linux" || process.platform === "darwin") {
    return unixManagedYtDlpPath();    // resolves via `which yt-dlp`
  }
  throw new Error(`Unsupported platform: ${process.platform}`);
}

PR #2 review summary

Change Windows-safe? Action
main.js refactoring Applied with fixes
extraResources.from: "electron/bin" Not applied
package-lock.json version bump Already at 1.0.10 on main
README docs ⚠️ broken markdown Rewritten

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 2 commits February 24, 2026 05:21
…ux/macOS

Safely refactors ensureManagedYtDlpPath() into platform-specific functions:
- winManagedYtDlpPath(): exact same logic as original (Windows unchanged)
- unixManagedYtDlpPath(): uses `which yt-dlp` for Linux/macOS
- ensureManagedYtDlpPath(): dispatches based on process.platform

Does NOT change extraResources.from (keeps "bin") to preserve Windows build.
Adds cross-platform build prerequisites to README.

Co-authored-by: Shripad735 <139649110+Shripad735@users.noreply.github.com>
…ions

Co-authored-by: Shripad735 <139649110+Shripad735@users.noreply.github.com>
Copilot AI changed the title [WIP] Review cross functionality for Windows compatibility Add cross-platform yt-dlp resolution without breaking Windows builds Feb 24, 2026
Copilot AI requested a review from Shripad735 February 24, 2026 05:25

This comment was marked as off-topic.

@Shripad735 Shripad735 closed this Feb 24, 2026
@Shripad735 Shripad735 deleted the copilot/review-cross-functionality-pr branch February 25, 2026 06:12
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.

2 participants