From 0821aea72b66a9c21de7e2b7fd80ae3b0a0d3165 Mon Sep 17 00:00:00 2001 From: Nexmoe <16796652+nexmoe@users.noreply.github.com> Date: Fri, 24 Oct 2025 19:51:25 +0800 Subject: [PATCH] refactor: streamline download path handling and remove unused output path logic --- src/main/download-engine.ts | 85 +----- src/main/ipc/services/file-system-service.ts | 149 ++++------ src/main/ipc/services/history-service.ts | 277 +----------------- .../src/components/download/DownloadItem.tsx | 101 +++---- src/renderer/src/locales/en.json | 1 - src/renderer/src/locales/zh.json | 1 - src/renderer/src/store/downloads.ts | 3 +- src/shared/types/index.ts | 4 +- 8 files changed, 126 insertions(+), 495 deletions(-) diff --git a/src/main/download-engine.ts b/src/main/download-engine.ts index 17a2830..bd32cb9 100644 --- a/src/main/download-engine.ts +++ b/src/main/download-engine.ts @@ -231,6 +231,7 @@ class DownloadEngine extends EventEmitter { } const createdAt = Date.now() + const settings = settingsManager.getAll() const item: DownloadItem = { id, @@ -247,7 +248,7 @@ class DownloadEngine extends EventEmitter { title: item.title, status: 'pending', downloadedAt: createdAt, - outputPath: options.outputPath + downloadPath: settings.downloadPath }) } @@ -255,7 +256,7 @@ class DownloadEngine extends EventEmitter { scopedLoggers.download.info('Starting download execution for ID:', id, 'URL:', options.url) const ytdlp = ytdlpManager.getInstance() const settings = settingsManager.getAll() - const downloadPath = options.outputPath || settings.downloadPath + const downloadPath = settings.downloadPath // Set environment variables for proper encoding on Windows if (process.platform === 'win32') { @@ -408,39 +409,8 @@ class DownloadEngine extends EventEmitter { } ) - // Handle yt-dlp events to capture output file path and format info - let actualOutputPath: string | null = null - + // Handle yt-dlp events to capture format info ytdlpProcess.on('ytDlpEvent', (eventType: string, eventData: string) => { - // Look for download destination messages - scopedLoggers.download.info('ytDlpEvent:', eventType, eventData) - if (eventType === 'download' && eventData.includes('Destination:')) { - const match = eventData.match(/Destination:\s*(.+)/) - if (match?.[1]) { - actualOutputPath = match[1].trim() - } - } - - // Also look for other output path patterns - if ( - eventType === 'download' && - (eventData.includes('has already been downloaded') || - eventData.includes('has already been downloaded')) - ) { - const match = eventData.match(/\[download\]\s*(.+?)\s+has already been downloaded/) - if (match?.[1]) { - actualOutputPath = match[1].trim() - } - } - - // Look for final output path in download messages - if (eventType === 'download' && eventData.includes('[download] 100%')) { - const match = eventData.match(/\[download\]\s*100%.*?of\s*(.+?)\s+at/) - if (match?.[1]) { - actualOutputPath = match[1].trim() - } - } - // Look for format selection messages if (eventType === 'info' && eventData.includes('format')) { // Extract format info from yt-dlp output @@ -484,31 +454,15 @@ class DownloadEngine extends EventEmitter { this.queue.downloadCompleted(id) if (code === 0) { - // Use actual output path from yt-dlp, or fallback to simple generated path - let finalOutputPath: string - if (actualOutputPath) { - finalOutputPath = actualOutputPath - scopedLoggers.download.info( - 'Using actual output path from yt-dlp for ID:', - id, - 'Path:', - finalOutputPath - ) - } else { - // Simple fallback: generate path based on video title and format - const title = videoInfo?.title || 'Unknown' - const sanitizedTitle = title.replace(/[<>:"/\\|?*]/g, '_').substring(0, 50) - const extension = - options.type === 'audio' ? options.extractFormat || 'mp3' : actualFormat || 'mp4' - const fileName = `${sanitizedTitle}.${extension}` - finalOutputPath = path.join(downloadPath, fileName) - scopedLoggers.download.warn( - 'Using fallback output path for ID:', - id, - 'Path:', - finalOutputPath - ) - } + // Generate file path using downloadPath + title + ext + const title = videoInfo?.title || 'Unknown' + const sanitizedTitle = title.replace(/[<>:"/\\|?*]/g, '_').substring(0, 50) + const extension = + options.type === 'audio' ? options.extractFormat || 'mp3' : actualFormat || 'mp4' + const fileName = `${sanitizedTitle}.${extension}` + const finalOutputPath = path.join(downloadPath, fileName) + + scopedLoggers.download.info('Generated file path for ID:', id, 'Path:', finalOutputPath) let fileSize: number | undefined try { @@ -529,7 +483,6 @@ class DownloadEngine extends EventEmitter { this.updateDownloadInfo(id, { status: 'completed', - outputPath: finalOutputPath, completedAt: Date.now(), fileSize, format: actualFormat || undefined, @@ -538,7 +491,7 @@ class DownloadEngine extends EventEmitter { }) scopedLoggers.download.info('Download completed successfully for ID:', id) this.emit('download-completed', id) - this.addToHistory(id, options, 'completed', undefined, finalOutputPath) + this.addToHistory(id, options, 'completed', undefined) } else { scopedLoggers.download.error( 'Download failed with exit code for ID:', @@ -617,9 +570,6 @@ class DownloadEngine extends EventEmitter { if (updates.duration !== undefined) { historyUpdates.duration = updates.duration } - if (updates.outputPath !== undefined) { - historyUpdates.outputPath = updates.outputPath - } if (updates.fileSize !== undefined) { historyUpdates.fileSize = updates.fileSize } @@ -669,20 +619,17 @@ class DownloadEngine extends EventEmitter { id: string, options: DownloadOptions, status: DownloadHistoryItem['status'], - error?: string, - actualOutputPath?: string + error?: string ): void { // Get the download item from the queue to get additional info const completedDownload = this.queue.getCompletedDownload(id) scopedLoggers.download.info('Completed download:', completedDownload) const completedAt = Date.now() - const finalOutputPath = actualOutputPath || options.outputPath this.upsertHistoryEntry(id, options, { title: completedDownload?.item.title || `Download ${id}`, thumbnail: completedDownload?.item.thumbnail, status, - outputPath: finalOutputPath, completedAt, error, duration: completedDownload?.item.duration, @@ -711,7 +658,7 @@ class DownloadEngine extends EventEmitter { thumbnail: updates.thumbnail, type: options.type, status: updates.status || 'pending', - outputPath: updates.outputPath, + downloadPath: updates.downloadPath, fileSize: updates.fileSize, duration: updates.duration, downloadedAt: updates.downloadedAt ?? Date.now(), diff --git a/src/main/ipc/services/file-system-service.ts b/src/main/ipc/services/file-system-service.ts index d512b3c..33f9399 100644 --- a/src/main/ipc/services/file-system-service.ts +++ b/src/main/ipc/services/file-system-service.ts @@ -1,5 +1,4 @@ import { execFile } from 'node:child_process' -import type { Dirent } from 'node:fs' import fs from 'node:fs/promises' import os from 'node:os' import path from 'node:path' @@ -61,12 +60,6 @@ class FileSystemService extends IpcService { } if (stats?.isDirectory()) { - const candidate = await this.findLikelyFile(normalizedPath, normalizedPath) - if (candidate) { - shell.showItemInFolder(candidate) - return true - } - const result = await shell.openPath(normalizedPath) if (result) { console.error('Failed to open directory:', result) @@ -75,43 +68,25 @@ class FileSystemService extends IpcService { return true } - const fallbackDirectory = path.dirname(normalizedPath) - const fallbackCandidate = await this.findLikelyFile(fallbackDirectory, normalizedPath) - if (fallbackCandidate) { - shell.showItemInFolder(fallbackCandidate) + // If the exact path doesn't exist, try to open the parent directory + const parentDirectory = path.dirname(normalizedPath) + const parentStats = await fs.stat(parentDirectory).catch(() => null) + + if (parentStats?.isDirectory()) { + const result = await shell.openPath(parentDirectory) + if (result) { + console.error('Failed to open parent directory:', result) + return false + } return true } - const result = await shell.openPath(fallbackDirectory) - if (!result) { - return true - } - console.error('Failed to open directory:', result) + console.error('File or directory does not exist:', normalizedPath) + return false } catch (error) { - try { - const directory = path.dirname(path.normalize(this.sanitizePath(filePath))) - const dirStats = await fs.stat(directory) - if (dirStats.isDirectory()) { - const fallbackCandidate = await this.findLikelyFile(directory, directory) - if (fallbackCandidate) { - shell.showItemInFolder(fallbackCandidate) - return true - } - const result = await shell.openPath(directory) - if (result) { - console.error('Failed to open directory:', result) - return false - } - return true - } - } catch (dirError) { - console.error('Failed to open parent directory:', dirError) - } console.error('Failed to open file location:', error) return false } - - return false } @IpcMethod() @@ -156,54 +131,6 @@ class FileSystemService extends IpcService { } } - private async findLikelyFile(directory: string, expectedPath: string): Promise { - try { - const dirStats = await fs.stat(directory) - if (!dirStats.isDirectory()) { - return null - } - - const entries = await fs.readdir(directory, { withFileTypes: true }) - const files = entries.filter((entry: Dirent) => entry.isFile()) - if (files.length === 0) { - return null - } - - const expectedBase = path.basename(expectedPath).toLowerCase() - const expectedName = path.parse(expectedPath).name.toLowerCase() - - const exactMatch = files.find((entry) => entry.name.toLowerCase() === expectedBase) - if (exactMatch) { - return path.join(directory, exactMatch.name) - } - - if (expectedName) { - const partialMatch = files.find((entry) => entry.name.toLowerCase().includes(expectedName)) - if (partialMatch) { - return path.join(directory, partialMatch.name) - } - } - - let latestMatch: { filePath: string; mtimeMs: number } | null = null - for (const entry of files) { - const candidatePath = path.join(directory, entry.name) - try { - const candidateStats = await fs.stat(candidatePath) - if (!latestMatch || candidateStats.mtimeMs > latestMatch.mtimeMs) { - latestMatch = { filePath: candidatePath, mtimeMs: candidateStats.mtimeMs } - } - } catch (statError) { - console.error('Failed to stat candidate file:', statError) - } - } - - return latestMatch?.filePath ?? null - } catch (error) { - console.error('Failed to search for matching file:', error) - return null - } - } - @IpcMethod() async openExternal(_context: IpcContext, url: string): Promise { try { @@ -288,6 +215,58 @@ class FileSystemService extends IpcService { .replace(/"/g, '"') .replace(/'/g, ''') } + + @IpcMethod() + async deleteFile(_context: IpcContext, filePath: string): Promise { + try { + if (!filePath) { + return false + } + + const sanitizedPath = this.sanitizePath(filePath) + const normalizedPath = path.normalize(sanitizedPath) + + const stats = await fs.stat(normalizedPath).catch((error) => { + const err = error as NodeJS.ErrnoException + if (err?.code === 'ENOENT') { + return null + } + throw error + }) + + if (!stats) { + return false + } + + if (stats.isFile()) { + await fs.unlink(normalizedPath).catch((error) => { + const err = error as NodeJS.ErrnoException + if (err?.code !== 'ENOENT') { + throw error + } + }) + return true + } + + if (stats.isDirectory()) { + const entries = await fs.readdir(normalizedPath) + if (entries.length === 0) { + await fs.rmdir(normalizedPath).catch((error) => { + const err = error as NodeJS.ErrnoException + if (err?.code !== 'ENOENT') { + throw error + } + }) + return true + } + } + + return false + } catch (error) { + console.error('Failed to delete file:', error) + return false + } + } } export { FileSystemService } diff --git a/src/main/ipc/services/history-service.ts b/src/main/ipc/services/history-service.ts index 5b8f620..f0f49d3 100644 --- a/src/main/ipc/services/history-service.ts +++ b/src/main/ipc/services/history-service.ts @@ -1,9 +1,6 @@ -import fs from 'node:fs/promises' -import path from 'node:path' import { type IpcContext, IpcMethod, IpcService } from 'electron-ipc-decorator' import type { DownloadHistoryItem } from '../../../shared/types' import { historyManager } from '../../lib/history-manager' -import { settingsManager } from '../../settings' class HistoryService extends IpcService { static readonly groupName = 'history' @@ -24,282 +21,10 @@ class HistoryService extends IpcService { } @IpcMethod() - async removeHistoryItem(_context: IpcContext, id: string, outputPath?: string): Promise { - const record = historyManager.getHistoryById(id) - - await this.deleteOutputResource(record, outputPath) - + removeHistoryItem(_context: IpcContext, id: string): boolean { return historyManager.removeHistoryItem(id) } - private async deleteOutputResource( - record: DownloadHistoryItem | undefined, - fallbackOutputPath?: string - ): Promise { - try { - const candidatePaths = new Set() - if (record?.outputPath) { - candidatePaths.add(record.outputPath) - } - if (fallbackOutputPath) { - candidatePaths.add(fallbackOutputPath) - } - - for (const candidate of candidatePaths) { - if (await this.tryDeletePath(candidate)) { - return - } - } - - const directories = this.collectCandidateDirectories(candidatePaths) - if (directories.length === 0) { - const defaultDownloadPath = settingsManager.get('downloadPath') - if (defaultDownloadPath) { - directories.push(defaultDownloadPath) - } - } - - const matchKeys = this.collectMatchKeys(record, candidatePaths) - if (matchKeys.length === 0) { - return - } - - const extensions = this.collectExtensions(candidatePaths, record) - for (const directory of directories) { - const matchedPath = await this.findMatchingFile(directory, matchKeys, extensions, record) - if (matchedPath && (await this.tryDeletePath(matchedPath))) { - return - } - } - } catch (error) { - console.error('Failed to delete output resource:', error) - } - } - - private sanitizePath(target: string): string { - return target.trim().replace(/^['"]|['"]$/g, '') - } - - private normalizeMatchString(value?: string): string { - if (!value) { - return '' - } - return value - .normalize('NFKC') - .toLowerCase() - .replace(/[^\p{L}\p{N}]+/gu, '') - } - - private collectCandidateDirectories(candidatePaths: Set): string[] { - const directories = new Set() - for (const candidate of candidatePaths) { - const sanitized = this.sanitizePath(candidate) - if (!sanitized) continue - const normalized = path.normalize(sanitized) - if (!path.isAbsolute(normalized)) continue - const directory = path.dirname(normalized) - if (directory) { - directories.add(directory) - } - } - return Array.from(directories) - } - - private collectExtensions( - candidatePaths: Set, - record: DownloadHistoryItem | undefined - ): Set { - const extensions = new Set() - - const addExtension = (ext?: string) => { - if (!ext) { - return - } - const trimmed = ext.trim() - if (!trimmed) { - return - } - const normalized = trimmed.startsWith('.') - ? trimmed.toLowerCase() - : `.${trimmed.toLowerCase()}` - extensions.add(normalized) - } - - for (const candidate of candidatePaths) { - const sanitized = this.sanitizePath(candidate) - if (!sanitized) continue - addExtension(path.extname(sanitized)) - } - - addExtension(record?.outputPath ? path.extname(record.outputPath) : undefined) - addExtension(record?.selectedFormat?.ext) - addExtension(record?.selectedFormat?.video_ext) - addExtension(record?.selectedFormat?.audio_ext) - - if (extensions.size === 0) { - const fallback = - record?.type === 'audio' - ? ['.mp3', '.m4a', '.aac', '.ogg', '.opus', '.flac', '.wav'] - : ['.mp4', '.mkv', '.webm', '.mov', '.avi'] - for (const ext of fallback) { - extensions.add(ext) - } - } - - return extensions - } - - private collectMatchKeys( - record: DownloadHistoryItem | undefined, - candidatePaths: Set - ): string[] { - const keys = new Set() - const fallbackKeys: string[] = [] - - const tryAddKey = (value?: string) => { - if (!value) return - const normalized = this.normalizeMatchString(value) - if (!normalized) return - if (normalized.length >= 3) { - keys.add(normalized) - } else { - fallbackKeys.push(normalized) - } - } - - for (const candidate of candidatePaths) { - const sanitized = this.sanitizePath(candidate) - if (!sanitized) continue - const baseName = path.parse(sanitized).name - tryAddKey(baseName) - } - - tryAddKey(record?.title) - tryAddKey(record?.id) - - if (keys.size === 0) { - for (const key of fallbackKeys) { - keys.add(key) - } - } - - return Array.from(keys) - } - - private async findMatchingFile( - directory: string, - matchKeys: string[], - extensions: Set, - record: DownloadHistoryItem | undefined - ): Promise { - try { - const dirStats = await fs.stat(directory).catch((error) => { - const err = error as NodeJS.ErrnoException - if (err?.code === 'ENOENT') { - return null - } - throw error - }) - - if (!dirStats || !dirStats.isDirectory()) { - return null - } - - const entries = await fs.readdir(directory, { withFileTypes: true }) - const matches: Array<{ path: string; diff: number }> = [] - const targetTimestamp = record?.completedAt ?? record?.downloadedAt - const maxDiff = 10 * 60 * 1000 // 10 minutes - - for (const entry of entries) { - if (!entry.isFile()) continue - - const entryPath = path.join(directory, entry.name) - const entryExt = path.extname(entry.name).toLowerCase() - if (extensions.size > 0 && entryExt && !extensions.has(entryExt)) { - continue - } - - const normalizedName = this.normalizeMatchString(entry.name) - if (!normalizedName && matchKeys.length > 0) continue - - const hasMatchKeys = matchKeys.length > 0 - const isMatch = hasMatchKeys - ? matchKeys.some((key) => key && normalizedName.includes(key)) - : true - if (!isMatch) continue - - const stats = await fs.stat(entryPath).catch(() => null) - if (!stats) continue - - if (targetTimestamp) { - const diff = Math.abs(stats.mtimeMs - targetTimestamp) - if (diff > maxDiff) { - continue - } - matches.push({ path: entryPath, diff }) - } else { - if (!hasMatchKeys) { - continue - } - matches.push({ path: entryPath, diff: Number.POSITIVE_INFINITY }) - } - } - - if (matches.length === 0) { - return null - } - - matches.sort((a, b) => a.diff - b.diff) - return matches[0]?.path ?? null - } catch (error) { - console.error('Failed to search for matching file:', error) - return null - } - } - - private async tryDeletePath(rawPath: string): Promise { - const sanitizedPath = this.sanitizePath(rawPath) - if (!sanitizedPath) return false - - const normalizedPath = path.normalize(sanitizedPath) - const stats = await fs.stat(normalizedPath).catch((error) => { - const err = error as NodeJS.ErrnoException - if (err?.code === 'ENOENT') { - return null - } - throw error - }) - - if (!stats) { - return false - } - - if (stats.isFile()) { - await fs.unlink(normalizedPath).catch((error) => { - const err = error as NodeJS.ErrnoException - if (err?.code !== 'ENOENT') { - throw error - } - }) - return true - } - - if (stats.isDirectory()) { - const entries = await fs.readdir(normalizedPath) - if (entries.length === 0) { - await fs.rmdir(normalizedPath).catch((error) => { - const err = error as NodeJS.ErrnoException - if (err?.code !== 'ENOENT') { - throw error - } - }) - return true - } - } - - return false - } - @IpcMethod() getHistoryCount(_context: IpcContext): { active: number diff --git a/src/renderer/src/components/download/DownloadItem.tsx b/src/renderer/src/components/download/DownloadItem.tsx index c7ec399..1f39eb3 100644 --- a/src/renderer/src/components/download/DownloadItem.tsx +++ b/src/renderer/src/components/download/DownloadItem.tsx @@ -3,18 +3,8 @@ import { Button } from '@renderer/components/ui/button' import { ImageWithPlaceholder } from '@renderer/components/ui/image-with-placeholder' import { Progress } from '@renderer/components/ui/progress' import { Tooltip, TooltipContent, TooltipTrigger } from '@renderer/components/ui/tooltip' -import { useSetAtom } from 'jotai' -import { - AlertCircle, - CheckCircle2, - Copy, - ExternalLink, - FolderOpen, - Loader2, - Play, - Trash2, - X -} from 'lucide-react' +import { useAtomValue, useSetAtom } from 'jotai' +import { AlertCircle, CheckCircle2, Copy, FolderOpen, Loader2, Play, Trash2, X } from 'lucide-react' import { useTranslation } from 'react-i18next' import { toast } from 'sonner' import { useCachedThumbnail } from '../../hooks/use-cached-thumbnail' @@ -24,6 +14,14 @@ import { removeDownloadAtom, removeHistoryRecordAtom } from '../../store/downloads' +import { settingsAtom } from '../../store/settings' + +// Helper function to generate file path with proper path separators +const generateFilePath = (downloadPath: string, title: string, format: string): string => { + const fileName = `${title}.${format}` + // Use proper path joining for cross-platform compatibility + return `${downloadPath}/${fileName}`.replace(/\//g, '\\') +} interface DownloadItemProps { download: DownloadRecord @@ -54,6 +52,7 @@ const formatDate = (timestamp?: number) => { export function DownloadItem({ download }: DownloadItemProps) { const { t } = useTranslation() + const settings = useAtomValue(settingsAtom) const removeDownload = useSetAtom(removeDownloadAtom) const removeHistory = useSetAtom(removeHistoryRecordAtom) const isHistory = download.entryType === 'history' @@ -76,14 +75,14 @@ export function DownloadItem({ download }: DownloadItemProps) { } } - const handleOpenFileLocation = async () => { - if (!download.outputPath) { - toast.error(t('notifications.openFolderFailed')) - return - } - + const handleOpenFolder = async () => { try { - const success = await ipcServices.fs.openFileLocation(download.outputPath) + // Generate file path using downloadPath + title + ext + const downloadPath = download.downloadPath || settings.downloadPath + const format = download.format || (download.type === 'audio' ? 'mp3' : 'mp4') + const filePath = generateFilePath(downloadPath, download.title, format) + + const success = await ipcServices.fs.openFileLocation(filePath) if (!success) { toast.error(t('notifications.openFolderFailed')) } @@ -92,33 +91,20 @@ export function DownloadItem({ download }: DownloadItemProps) { toast.error(t('notifications.openFolderFailed')) } } - - const handleOpenFile = async () => { - if (!download.outputPath) { - toast.error(t('notifications.openFileFailed')) - return - } - - try { - await ipcServices.fs.openFileLocation(download.outputPath) - } catch (error) { - console.error('Failed to open file:', error) - toast.error(t('notifications.openFileFailed')) - } - } - - const handleOpenFolder = async () => { - await handleOpenFileLocation() - } - + // need title, downloadPath, format const handleCopyToClipboard = async () => { - if (!download.outputPath) { + if (!download.title || !download.downloadPath || !download.format) { toast.error(t('notifications.copyFailed')) return } try { - const success = await ipcServices.fs.copyFileToClipboard(download.outputPath) + // Generate file path using downloadPath + title + ext + const downloadPath = download.downloadPath + const format = download.format + const filePath = generateFilePath(downloadPath, download.title, format) + + const success = await ipcServices.fs.copyFileToClipboard(filePath) if (!success) { toast.error(t('notifications.copyFailed')) return @@ -130,10 +116,21 @@ export function DownloadItem({ download }: DownloadItemProps) { } } + // need id const handleRemoveHistory = async () => { if (!isHistory) return try { - await ipcServices.history.removeHistoryItem(download.id, download.outputPath) + // Generate file path using downloadPath + title + ext + const downloadPath = download.downloadPath || settings.downloadPath + const format = download.format || (download.type === 'audio' ? 'mp3' : 'mp4') + const filePath = generateFilePath(downloadPath, download.title, format) + + // Remove from history first + await ipcServices.history.removeHistoryItem(download.id) + + // Then try to delete the file + await ipcServices.fs.deleteFile(filePath) + removeHistory(download.id) toast.success(t('notifications.itemRemoved')) } catch (error) { @@ -152,7 +149,7 @@ export function DownloadItem({ download }: DownloadItemProps) { case 'processing': return case 'pending': - return + return case 'cancelled': return default: @@ -290,7 +287,7 @@ export function DownloadItem({ download }: DownloadItemProps) {
{isHistory ? ( <> - {download.outputPath && ( + {download.status === 'completed' && ( <> @@ -299,6 +296,7 @@ export function DownloadItem({ download }: DownloadItemProps) { size="icon" className="h-8 w-8 shrink-0" onClick={handleCopyToClipboard} + disabled={!download.title || !download.downloadPath || !download.format} > @@ -342,23 +340,8 @@ export function DownloadItem({ download }: DownloadItemProps) { ) : ( <> - {download.status === 'completed' && download.outputPath && ( + {download.status === 'completed' && ( <> - - - - - -

{t('history.openFile')}

- -