-
Notifications
You must be signed in to change notification settings - Fork 963
feat(desktop): improve ringtone settings with duration display and bug fixes #318
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7b40d74
c94bebe
8232d97
504f7df
73f97fb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,177 @@ | ||||||||||||||||||||||||||||||||||||||
| import type { ChildProcess } from "node:child_process"; | ||||||||||||||||||||||||||||||||||||||
| import { execFile } from "node:child_process"; | ||||||||||||||||||||||||||||||||||||||
| import { existsSync, readdirSync } from "node:fs"; | ||||||||||||||||||||||||||||||||||||||
| import { join } from "node:path"; | ||||||||||||||||||||||||||||||||||||||
| import { app } from "electron"; | ||||||||||||||||||||||||||||||||||||||
| import { z } from "zod"; | ||||||||||||||||||||||||||||||||||||||
| import { publicProcedure, router } from "../.."; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||
| * Track current playing session to handle race conditions. | ||||||||||||||||||||||||||||||||||||||
| * Each play operation gets a unique session ID. When stop is called, | ||||||||||||||||||||||||||||||||||||||
| * the session is invalidated so any pending fallback processes won't start. | ||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||
| let currentSession: { | ||||||||||||||||||||||||||||||||||||||
| id: number; | ||||||||||||||||||||||||||||||||||||||
| process: ChildProcess | null; | ||||||||||||||||||||||||||||||||||||||
| } | null = null; | ||||||||||||||||||||||||||||||||||||||
| let nextSessionId = 0; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||
| * Gets the path to a ringtone sound file. | ||||||||||||||||||||||||||||||||||||||
| * In development, reads from src/resources. In production, reads from the bundled resources. | ||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||
| function getRingtonePath(filename: string): string { | ||||||||||||||||||||||||||||||||||||||
| const isDev = !app.isPackaged; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| if (isDev) { | ||||||||||||||||||||||||||||||||||||||
| return join(app.getAppPath(), "src/resources/sounds", filename); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| return join(process.resourcesPath, "resources/sounds", filename); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||
| * Gets the directory containing ringtone files | ||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||
| function getRingtonesDirectory(): string { | ||||||||||||||||||||||||||||||||||||||
| const isDev = !app.isPackaged; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| if (isDev) { | ||||||||||||||||||||||||||||||||||||||
| return join(app.getAppPath(), "src/resources/sounds"); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| return join(process.resourcesPath, "resources/sounds"); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||
| * Stops the currently playing sound and invalidates the session | ||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||
| function stopCurrentSound(): void { | ||||||||||||||||||||||||||||||||||||||
| if (currentSession) { | ||||||||||||||||||||||||||||||||||||||
| if (currentSession.process) { | ||||||||||||||||||||||||||||||||||||||
| // Use SIGKILL for immediate termination (afplay doesn't always respond to SIGTERM) | ||||||||||||||||||||||||||||||||||||||
| currentSession.process.kill("SIGKILL"); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| currentSession = null; | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||
| * Plays a sound file using platform-specific commands. | ||||||||||||||||||||||||||||||||||||||
| * Uses session tracking to prevent race conditions with fallback audio players. | ||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||
| function playSoundFile(soundPath: string): void { | ||||||||||||||||||||||||||||||||||||||
| if (!existsSync(soundPath)) { | ||||||||||||||||||||||||||||||||||||||
| console.warn(`[ringtone] Sound file not found: ${soundPath}`); | ||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| // Stop any currently playing sound first | ||||||||||||||||||||||||||||||||||||||
| stopCurrentSound(); | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| // Create a new session for this play operation | ||||||||||||||||||||||||||||||||||||||
| const sessionId = nextSessionId++; | ||||||||||||||||||||||||||||||||||||||
| currentSession = { id: sessionId, process: null }; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| if (process.platform === "darwin") { | ||||||||||||||||||||||||||||||||||||||
| currentSession.process = execFile("afplay", [soundPath], () => { | ||||||||||||||||||||||||||||||||||||||
| // Only clear if this session is still active | ||||||||||||||||||||||||||||||||||||||
| if (currentSession?.id === sessionId) { | ||||||||||||||||||||||||||||||||||||||
| currentSession = null; | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
| } else if (process.platform === "win32") { | ||||||||||||||||||||||||||||||||||||||
| currentSession.process = execFile( | ||||||||||||||||||||||||||||||||||||||
| "powershell", | ||||||||||||||||||||||||||||||||||||||
| ["-c", `(New-Object Media.SoundPlayer '${soundPath}').PlaySync()`], | ||||||||||||||||||||||||||||||||||||||
| () => { | ||||||||||||||||||||||||||||||||||||||
| if (currentSession?.id === sessionId) { | ||||||||||||||||||||||||||||||||||||||
| currentSession = null; | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+82
to
+91
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Potential command injection via PowerShell. The Consider escaping the path or using a safer invocation pattern: } else if (process.platform === "win32") {
+ // Escape single quotes in path for PowerShell
+ const escapedPath = soundPath.replace(/'/g, "''");
currentPlayingProcess = execFile(
"powershell",
- ["-c", `(New-Object Media.SoundPlayer '${soundPath}').PlaySync()`],
+ ["-c", `(New-Object Media.SoundPlayer '${escapedPath}').PlaySync()`],
() => {
currentPlayingProcess = null;
},
);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||
| // Linux - try common audio players with race-safe fallback | ||||||||||||||||||||||||||||||||||||||
| currentSession.process = execFile("paplay", [soundPath], (error) => { | ||||||||||||||||||||||||||||||||||||||
| // Check if this session is still active before proceeding | ||||||||||||||||||||||||||||||||||||||
| if (currentSession?.id !== sessionId) { | ||||||||||||||||||||||||||||||||||||||
| return; // Session was stopped, don't start fallback | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| if (error) { | ||||||||||||||||||||||||||||||||||||||
| // paplay failed, try aplay as fallback | ||||||||||||||||||||||||||||||||||||||
| currentSession.process = execFile("aplay", [soundPath], () => { | ||||||||||||||||||||||||||||||||||||||
| if (currentSession?.id === sessionId) { | ||||||||||||||||||||||||||||||||||||||
| currentSession = null; | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||
| currentSession = null; | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||
| * Ringtone router for audio preview and playback operations | ||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||
| export const createRingtoneRouter = () => { | ||||||||||||||||||||||||||||||||||||||
| return router({ | ||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||
| * Preview a ringtone sound by filename | ||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||
| preview: publicProcedure | ||||||||||||||||||||||||||||||||||||||
| .input(z.object({ filename: z.string() })) | ||||||||||||||||||||||||||||||||||||||
| .mutation(({ input }) => { | ||||||||||||||||||||||||||||||||||||||
| // Handle "none" case - no sound | ||||||||||||||||||||||||||||||||||||||
| if (!input.filename || input.filename === "") { | ||||||||||||||||||||||||||||||||||||||
| return { success: true as const }; | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| const soundPath = getRingtonePath(input.filename); | ||||||||||||||||||||||||||||||||||||||
| playSoundFile(soundPath); | ||||||||||||||||||||||||||||||||||||||
| return { success: true as const }; | ||||||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||
| * Stop the currently playing ringtone preview | ||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||
| stop: publicProcedure.mutation(() => { | ||||||||||||||||||||||||||||||||||||||
| stopCurrentSound(); | ||||||||||||||||||||||||||||||||||||||
| return { success: true as const }; | ||||||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||
| * Get the list of available ringtone files from the sounds directory | ||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||
| list: publicProcedure.query(() => { | ||||||||||||||||||||||||||||||||||||||
| const ringtonesDir = getRingtonesDirectory(); | ||||||||||||||||||||||||||||||||||||||
| const files: string[] = []; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| // Add ringtones from the sounds directory if it exists | ||||||||||||||||||||||||||||||||||||||
| if (existsSync(ringtonesDir)) { | ||||||||||||||||||||||||||||||||||||||
| const dirFiles = readdirSync(ringtonesDir).filter( | ||||||||||||||||||||||||||||||||||||||
| (file) => | ||||||||||||||||||||||||||||||||||||||
| file.endsWith(".mp3") || | ||||||||||||||||||||||||||||||||||||||
| file.endsWith(".wav") || | ||||||||||||||||||||||||||||||||||||||
| file.endsWith(".ogg"), | ||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||
| files.push(...dirFiles); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| return files; | ||||||||||||||||||||||||||||||||||||||
| }), | ||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||
| * Plays the notification sound based on the selected ringtone. | ||||||||||||||||||||||||||||||||||||||
| * This is used by the notification system. | ||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||
| export function playNotificationRingtone(filename: string): void { | ||||||||||||||||||||||||||||||||||||||
| if (!filename || filename === "") { | ||||||||||||||||||||||||||||||||||||||
| return; // No sound for "none" option | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| const soundPath = getRingtonePath(filename); | ||||||||||||||||||||||||||||||||||||||
| playSoundFile(soundPath); | ||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SIGKILL doesn't exist on Windows.
SIGKILLis a Unix signal and will not terminate processes on Windows. Thekill()method on Windows requiresSIGTERMor no argument (defaults toSIGTERM), and even then behavior differs.Consider using a cross-platform approach:
function stopCurrentSound(): void { if (currentPlayingProcess) { - // Use SIGKILL for immediate termination (afplay doesn't always respond to SIGTERM) - currentPlayingProcess.kill("SIGKILL"); + // Use platform-appropriate signal for termination + if (process.platform === "win32") { + currentPlayingProcess.kill(); // Windows defaults to SIGTERM-like behavior + } else { + currentPlayingProcess.kill("SIGKILL"); // Unix: immediate termination + } currentPlayingProcess = null; } }📝 Committable suggestion
🤖 Prompt for AI Agents