Conversation
this is a purpose for an switch button for switch into download method with yt-dlp et youtube.js
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -1,8 +1,9 @@ | |||
| import { existsSync, mkdirSync, writeFileSync } from 'node:fs'; | |||
| import { existsSync, mkdirSync, writeFileSync, readdirSync } from 'node:fs'; | |||
There was a problem hiding this comment.
'readdirSync' is defined but never used.
| import { randomBytes } from 'node:crypto'; | ||
|
|
||
| import { app, type BrowserWindow, dialog, ipcMain } from 'electron'; | ||
| import { spawn, spawnSync } from 'node:child_process'; |
There was a problem hiding this comment.
🚫 [eslint] <importPlugin/order> reported by reviewdog 🐶
There should be at least one empty line between import groups
| import { spawn, spawnSync } from 'node:child_process'; | |
| import { spawn, spawnSync } from 'node:child_process'; | |
| import { app, type BrowserWindow, dialog, ipcMain } from 'electron'; | ||
| import { spawn, spawnSync } from 'node:child_process'; |
There was a problem hiding this comment.
🚫 [eslint] <importPlugin/order> reported by reviewdog 🐶node:child_process import should occur before import of electron
| import { app, type BrowserWindow, dialog, ipcMain } from 'electron'; | |
| import { spawn, spawnSync } from 'node:child_process'; | |
| import { spawn, spawnSync } from 'node:child_process'; | |
| import { app, type BrowserWindow, dialog, ipcMain } from 'electron'; |
|
i dont have say'd why i have code that its because for some reason i dont know why but youtube.js method won't work somtime with me |
src/plugins/downloader/main/index.ts
Outdated
| const candidates: (string | undefined)[] = []; | ||
| if (preferred) candidates.push(preferred); | ||
| candidates.push('/usr/bin/yt-dlp', '/usr/local/bin/yt-dlp', 'yt-dlp'); |
There was a problem hiding this comment.
Instead of having a hardcoded path of candidates, we should instead be searching based on PATH.
Something like which could be used.
Also, yt-dlp is a fork, it'd probably be a good idea to also support the original youtube-dl
There was a problem hiding this comment.
i can try but normaly its over my comptence
There was a problem hiding this comment.
but honesly i have place an config on the extention configuration for choose the path with the path finder of the system file explorer avalible on the system personaly i think that probably good
There was a problem hiding this comment.
Also, yt-dlp is a fork, it'd probably be a good idea to also support the original youtube-dl
Supporting both is welcomed. But, if only 1 can be supported in this PR, I'll prefer yt-dlp. The original youtube-dl doesn't even get packaged by any Linux distributions anymore.
There was a problem hiding this comment.
i can try but normaly its over my comptence
pnpm add which
pnpm add -D @types/whichimport { which } from 'which';
const ytdlpExecutable = await which('yt-dlp', { nothrow: true })this is over your competence? I beg to differ
There was a problem hiding this comment.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
eslint
🚫 [eslint] <prettier/prettier> reported by reviewdog 🐶
Replace 'plugins.downloader.backend.dialog.ytdlp-permission.message' with ⏎··················'plugins.downloader.backend.dialog.ytdlp-permission.message',⏎················
🚫 [eslint] <@typescript-eslint/prefer-promise-reject-errors> reported by reviewdog 🐶
Expected the Promise rejection reason to be an Error.
🚫 [eslint] <importPlugin/order> reported by reviewdog 🐶
There should be at least one empty line between import groups
🚫 [eslint] <importPlugin/order> reported by reviewdog 🐶node:child_process import should occur before import of electron
'spawn' is defined but never used.
🚫 [eslint] <prettier/prettier> reported by reviewdog 🐶
Delete ··
🚫 [eslint] <prettier/prettier> reported by reviewdog 🐶
Delete ····
🚫 [eslint] <@typescript-eslint/no-unsafe-assignment> reported by reviewdog 🐶
Unsafe assignment of an error typed value.
🚫 [eslint] <@typescript-eslint/no-unsafe-call> reported by reviewdog 🐶
Unsafe call of a(n) error type typed value.
🚫 [eslint] <@typescript-eslint/no-unsafe-return> reported by reviewdog 🐶
Unsafe return of a value of type error.
'_' is defined but never used.
🚫 [eslint] <prettier/prettier> reported by reviewdog 🐶
Replace ·encoding:·'utf8',·stdio:·'ignore' with ⏎··········encoding:·'utf8',⏎··········stdio:·'ignore',⏎·······
'_' is defined but never used.
🚫 [eslint] <prettier/prettier> reported by reviewdog 🐶
Delete ····
🚫 [eslint] <@typescript-eslint/no-unsafe-assignment> reported by reviewdog 🐶
Unsafe assignment of an error typed value.
🚫 [eslint] <@typescript-eslint/no-unsafe-call> reported by reviewdog 🐶
Unsafe call of a(n) error type typed value.
🚫 [eslint] <@typescript-eslint/no-unsafe-return> reported by reviewdog 🐶
Unsafe return of a value of type error.
'_' is defined but never used.
🚫 [eslint] <prettier/prettier> reported by reviewdog 🐶
Delete ····
🚫 [eslint] <prettier/prettier> reported by reviewdog 🐶
Delete ··
🚫 [eslint] <prettier/prettier> reported by reviewdog 🐶
Insert ⏎···
🚫 [eslint] <prettier/prettier> reported by reviewdog 🐶
Insert ⏎···
🚫 [eslint] <@typescript-eslint/no-unsafe-assignment> reported by reviewdog 🐶
Unsafe assignment of an error typed value.
pear-desktop/src/plugins/downloader/menu.ts
Line 288 in 58b75d2
🚫 [eslint] <prettier/prettier> reported by reviewdog 🐶
Replace config.ytdlpPath·??·undefined with ⏎··············config.ytdlpPath·??·undefined,⏎············
pear-desktop/src/plugins/downloader/menu.ts
Line 288 in 58b75d2
🚫 [eslint] <@typescript-eslint/no-unsafe-assignment> reported by reviewdog 🐶
Unsafe assignment of an error typed value.
pear-desktop/src/plugins/downloader/menu.ts
Line 289 in 58b75d2
🚫 [eslint] <prettier/prettier> reported by reviewdog 🐶
Replace config.ytdlpFfmpegPath·??·undefined); with ⏎··············config.ytdlpFfmpegPath·??·undefined,
pear-desktop/src/plugins/downloader/menu.ts
Line 289 in 58b75d2
🚫 [eslint] <prettier/prettier> reported by reviewdog 🐶
Insert );⏎
pear-desktop/src/plugins/downloader/menu.ts
Line 290 in 58b75d2
🚫 [eslint] <prettier/prettier> reported by reviewdog 🐶
Replace "yt-dlp·not·found.·Please·install·yt-dlp·or·provide·the·path·in·the·settings." with 'yt-dlp·not·found.·Please·install·yt-dlp·or·provide·the·path·in·the·settings.'
pear-desktop/src/plugins/downloader/menu.ts
Line 297 in 58b75d2
🚫 [eslint] <stylistic/quotes> reported by reviewdog 🐶
Strings must use singlequote.
pear-desktop/src/plugins/downloader/menu.ts
Line 297 in 58b75d2
🚫 [eslint] <prettier/prettier> reported by reviewdog 🐶
Delete ············
pear-desktop/src/plugins/downloader/menu.ts
Line 301 in 58b75d2
🚫 [eslint] <prettier/prettier> reported by reviewdog 🐶
Replace "ffmpeg·not·found.·Please·install·ffmpeg·or·provide·the·path·in·the·settings." with 'ffmpeg·not·found.·Please·install·ffmpeg·or·provide·the·path·in·the·settings.'
pear-desktop/src/plugins/downloader/menu.ts
Line 308 in 58b75d2
🚫 [eslint] <stylistic/quotes> reported by reviewdog 🐶
Strings must use singlequote.
pear-desktop/src/plugins/downloader/menu.ts
Line 308 in 58b75d2
🚫 [eslint] <prettier/prettier> reported by reviewdog 🐶
Delete ············
pear-desktop/src/plugins/downloader/menu.ts
Line 312 in 58b75d2
| const findYtdlpExecutable = async (preferred?: string) => { | ||
| const candidates: (string | undefined)[] = []; | ||
| if (preferred) candidates.push(preferred); | ||
|
|
There was a problem hiding this comment.
🚫 [eslint] <prettier/prettier> reported by reviewdog 🐶
Delete ······
|
|
||
| // Try to find yt-dlp using the which command | ||
| try { | ||
| const foundPath = await which('yt-dlp', { nothrow: true }); |
There was a problem hiding this comment.
🚫 [eslint] <@typescript-eslint/no-unsafe-assignment> reported by reviewdog 🐶
Unsafe assignment of an error typed value.
|
|
||
| // Try to find yt-dlp using the which command | ||
| try { | ||
| const foundPath = await which('yt-dlp', { nothrow: true }); |
There was a problem hiding this comment.
🚫 [eslint] <@typescript-eslint/no-unsafe-call> reported by reviewdog 🐶
Unsafe call of a(n) error type typed value.
| // Try to find yt-dlp using the which command | ||
| try { | ||
| const foundPath = await which('yt-dlp', { nothrow: true }); | ||
| if (foundPath) return foundPath; |
There was a problem hiding this comment.
🚫 [eslint] <@typescript-eslint/no-unsafe-return> reported by reviewdog 🐶
Unsafe return of a value of type error.
| try { | ||
| const foundPath = await which('yt-dlp', { nothrow: true }); | ||
| if (foundPath) return foundPath; | ||
| } catch (_) { |
There was a problem hiding this comment.
'_' is defined but never used.
| const args: string[] = ['--no-playlist', '--add-metadata', '--embed-thumbnail', '--output', outputTemplate]; | ||
|
|
||
| if (targetFileExtension === 'mp3') { | ||
| args.unshift('--extract-audio', '--audio-format', 'mp3', '--audio-quality', '0'); |
There was a problem hiding this comment.
🚫 [eslint] <prettier/prettier> reported by reviewdog 🐶
Replace '--extract-audio',·'--audio-format',·'mp3',·'--audio-quality',·'0' with ⏎········'--extract-audio',⏎········'--audio-format',⏎········'mp3',⏎········'--audio-quality',⏎········'0',⏎······
| args.unshift('--extract-audio', '--audio-format', 'mp3', '--audio-quality', '0'); | |
| args.unshift( | |
| '--extract-audio', | |
| '--audio-format', | |
| 'mp3', | |
| '--audio-quality', | |
| '0', | |
| ); |
| } | ||
|
|
||
| // Pass ffmpeg location to yt-dlp | ||
| args.push('--ffmpeg-location', ffmpegPath); |
There was a problem hiding this comment.
🚫 [eslint] <@typescript-eslint/no-unsafe-argument> reported by reviewdog 🐶
Unsafe argument of type error typed assigned to a parameter of type string.
|
|
||
| await new Promise<void>((resolve, reject) => { | ||
| try { | ||
| const proc = spawn(ytdlpExecutable, [...args, urlToDownload], { |
There was a problem hiding this comment.
🚫 [eslint] <@typescript-eslint/no-unsafe-argument> reported by reviewdog 🐶
Unsafe argument of type error typed assigned to a parameter of type string.
| defaultId: 0, | ||
| title: | ||
| t('plugins.downloader.backend.dialog.ytdlp-error.title') || | ||
| `yt-dlp erreur`, |
There was a problem hiding this comment.
🚫 [eslint] <stylistic/quotes> reported by reviewdog 🐶
Strings must use singlequote.
| `yt-dlp erreur`, | |
| 'yt-dlp erreur', |
| `Derniers messages d'erreur:\n${tail(err)}\n\nSortie:\n${tail(out)}`, | ||
| }); | ||
|
|
||
| reject(new Error(`yt-dlp exited with code ${code}: ${tail(err, 500)}`)); |
There was a problem hiding this comment.
🚫 [eslint] <prettier/prettier> reported by reviewdog 🐶
Replace new·Error(yt-dlp·exited·with·code·${code}:·${tail(err,·500)}) with ⏎············new·Error(yt-dlp·exited·with·code·${code}:·${tail(err,·500)}),⏎··········
| reject(new Error(`yt-dlp exited with code ${code}: ${tail(err, 500)}`)); | |
| reject( | |
| new Error(`yt-dlp exited with code ${code}: ${tail(err, 500)}`), | |
| ); |
3 participants
this is a purpose for an switch button for switch into download method with yt-dlp et youtube.js
i think my code isn't the best code but with my test all working i have tested on linux i can't test it on windows rip but all working fine with me