From d43800f19a6c96307e5f35acbdaa3a0fe384d443 Mon Sep 17 00:00:00 2001 From: 4gray Date: Wed, 13 May 2026 23:48:29 +0200 Subject: [PATCH] fix(settings): address external player argument review --- .../src/app/events/player.events.ts | 19 +++-------- .../src/app/events/settings.events.ts | 24 ++------------ .../src/app/settings/settings.component.ts | 27 +++------------ .../src/lib/settings-store.service.ts | 4 +-- libs/shared/interfaces/src/index.ts | 1 + .../external-player-arguments.utils.spec.ts | 33 +++++++++++++++++++ .../lib/external-player-arguments.utils.ts | 28 ++++++++++++++++ 7 files changed, 76 insertions(+), 60 deletions(-) create mode 100644 libs/shared/interfaces/src/lib/external-player-arguments.utils.spec.ts create mode 100644 libs/shared/interfaces/src/lib/external-player-arguments.utils.ts diff --git a/apps/electron-backend/src/app/events/player.events.ts b/apps/electron-backend/src/app/events/player.events.ts index 20aea4bcd..3b3e66838 100644 --- a/apps/electron-backend/src/app/events/player.events.ts +++ b/apps/electron-backend/src/app/events/player.events.ts @@ -4,6 +4,8 @@ import { EXTERNAL_PLAYER_SESSION_UPDATE, ExternalPlayerName, ExternalPlayerSession, + parseExternalPlayerArguments, + type ExternalPlayerArgumentsInput, } from 'shared-interfaces'; import App from '../app'; import { @@ -207,23 +209,10 @@ export function buildExternalPlayerSpawnSpec( }; } -export function parseExternalPlayerArguments( - value: string | string[] | null | undefined -): string[] { - if (Array.isArray(value)) { - return value.map((argument) => argument.trim()).filter(Boolean); - } - - return ( - value - ?.split(/\r?\n/) - .map((argument) => argument.trim()) - .filter(Boolean) ?? [] - ); -} +export { parseExternalPlayerArguments }; export function buildPlayerArgsWithCustomArguments( - customArguments: string | string[] | null | undefined, + customArguments: ExternalPlayerArgumentsInput, playerArgs: string[] ): string[] { return [...parseExternalPlayerArguments(customArguments), ...playerArgs]; diff --git a/apps/electron-backend/src/app/events/settings.events.ts b/apps/electron-backend/src/app/events/settings.events.ts index 2216cd691..ee35552a2 100644 --- a/apps/electron-backend/src/app/events/settings.events.ts +++ b/apps/electron-backend/src/app/events/settings.events.ts @@ -1,4 +1,5 @@ import { ipcMain } from 'electron'; +import { normalizeExternalPlayerArguments } from 'shared-interfaces'; import { MPV_PLAYER_ARGUMENTS, MPV_REUSE_INSTANCE, @@ -14,39 +15,20 @@ export default class SettingsEvents { } } -function normalizeExternalPlayerArgumentsForStore(value: unknown): string { - if (Array.isArray(value)) { - return value - .map((argument) => String(argument).trim()) - .filter(Boolean) - .join('\n'); - } - - if (typeof value !== 'string') { - return ''; - } - - return value - .split(/\r?\n/) - .map((argument) => argument.trim()) - .filter(Boolean) - .join('\n'); -} - ipcMain.handle('SETTINGS_UPDATE', (_event, arg) => { console.log('Received SETTINGS_UPDATE with data:', arg); if (arg.mpvPlayerArguments !== undefined) { store.set( MPV_PLAYER_ARGUMENTS, - normalizeExternalPlayerArgumentsForStore(arg.mpvPlayerArguments) + normalizeExternalPlayerArguments(arg.mpvPlayerArguments) ); } if (arg.vlcPlayerArguments !== undefined) { store.set( VLC_PLAYER_ARGUMENTS, - normalizeExternalPlayerArgumentsForStore(arg.vlcPlayerArguments) + normalizeExternalPlayerArguments(arg.vlcPlayerArguments) ); } diff --git a/apps/web/src/app/settings/settings.component.ts b/apps/web/src/app/settings/settings.component.ts index b1d417f21..61c48f3b1 100644 --- a/apps/web/src/app/settings/settings.component.ts +++ b/apps/web/src/app/settings/settings.component.ts @@ -47,6 +47,7 @@ import { EmbeddedMpvSupport, CoverSize, Language, + normalizeExternalPlayerArguments, StartupBehavior, StreamFormat, Theme, @@ -475,19 +476,13 @@ export class SettingsComponent implements OnInit, OnDestroy { vlcPlayerPath: this.normalizeExternalPlayerPath( this.settingsForm.value.vlcPlayerPath ), - mpvPlayerArguments: this.normalizeExternalPlayerArguments( + mpvPlayerArguments: normalizeExternalPlayerArguments( this.settingsForm.value.mpvPlayerArguments ), - vlcPlayerArguments: this.normalizeExternalPlayerArguments( + vlcPlayerArguments: normalizeExternalPlayerArguments( this.settingsForm.value.vlcPlayerArguments ), }; - const mpvPlayerPath = this.normalizeExternalPlayerPath( - settings.mpvPlayerPath - ); - const vlcPlayerPath = this.normalizeExternalPlayerPath( - settings.vlcPlayerPath - ); this.settingsStore.updateSettings(settings).then(() => { this.applyChangedSettings(); @@ -495,8 +490,8 @@ export class SettingsComponent implements OnInit, OnDestroy { if (window.electron) { window.electron.updateSettings(settings); - window.electron.setMpvPlayerPath(mpvPlayerPath); - window.electron.setVlcPlayerPath(vlcPlayerPath); + window.electron.setMpvPlayerPath(settings.mpvPlayerPath); + window.electron.setVlcPlayerPath(settings.vlcPlayerPath); } }); if (this.isDialog) { @@ -510,18 +505,6 @@ export class SettingsComponent implements OnInit, OnDestroy { return playerPath?.trim() ?? ''; } - private normalizeExternalPlayerArguments( - playerArguments: string | null | undefined - ): string { - return ( - playerArguments - ?.split(/\r?\n/) - .map((argument) => argument.trim()) - .filter(Boolean) - .join('\n') ?? '' - ); - } - /** * Applies the changed settings to the app */ diff --git a/libs/services/src/lib/settings-store.service.ts b/libs/services/src/lib/settings-store.service.ts index ae02ff58a..dc290a0cc 100644 --- a/libs/services/src/lib/settings-store.service.ts +++ b/libs/services/src/lib/settings-store.service.ts @@ -138,10 +138,10 @@ export const SettingsStore = signalStore( showExternalPlaybackBar: store.showExternalPlaybackBar!(), theme: store.theme(), mpvPlayerPath: store.mpvPlayerPath(), - mpvPlayerArguments: store.mpvPlayerArguments?.() ?? '', + mpvPlayerArguments: store.mpvPlayerArguments() ?? '', mpvReuseInstance: store.mpvReuseInstance(), vlcPlayerPath: store.vlcPlayerPath(), - vlcPlayerArguments: store.vlcPlayerArguments?.() ?? '', + vlcPlayerArguments: store.vlcPlayerArguments() ?? '', vlcReuseInstance: store.vlcReuseInstance(), remoteControl: store.remoteControl(), remoteControlPort: store.remoteControlPort(), diff --git a/libs/shared/interfaces/src/index.ts b/libs/shared/interfaces/src/index.ts index ac1cb4fbd..02552fbb7 100644 --- a/libs/shared/interfaces/src/index.ts +++ b/libs/shared/interfaces/src/index.ts @@ -6,6 +6,7 @@ export * from './lib/epg-channel-with-programs.interface'; export * from './lib/epg-channel.model'; export * from './lib/epg-item.interface'; export * from './lib/epg-program.model'; +export * from './lib/external-player-arguments.utils'; export * from './lib/external-player-session.interface'; export * from './lib/indexed-db.config'; export * from './lib/ipc-command.class'; diff --git a/libs/shared/interfaces/src/lib/external-player-arguments.utils.spec.ts b/libs/shared/interfaces/src/lib/external-player-arguments.utils.spec.ts new file mode 100644 index 000000000..0942451c9 --- /dev/null +++ b/libs/shared/interfaces/src/lib/external-player-arguments.utils.spec.ts @@ -0,0 +1,33 @@ +import { + normalizeExternalPlayerArguments, + parseExternalPlayerArguments, +} from './external-player-arguments.utils'; + +describe('external player arguments utils', () => { + it('parses one argument per non-empty trimmed line', () => { + expect( + parseExternalPlayerArguments( + ' --screen=1\n\n--geometry=1280x720\r\n --hwdec=auto-safe ' + ) + ).toEqual(['--screen=1', '--geometry=1280x720', '--hwdec=auto-safe']); + }); + + it('normalizes arguments to newline-separated trimmed lines', () => { + expect( + normalizeExternalPlayerArguments([ + ' --screen=1 ', + '', + ' --geometry=1280x720 ', + ]) + ).toBe('--screen=1\n--geometry=1280x720'); + }); + + it('treats missing or unsupported values as empty arguments', () => { + expect(parseExternalPlayerArguments(undefined)).toEqual([]); + expect(parseExternalPlayerArguments(null)).toEqual([]); + expect(parseExternalPlayerArguments(123 as unknown as string)).toEqual( + [] + ); + expect(normalizeExternalPlayerArguments(' \n ')).toBe(''); + }); +}); diff --git a/libs/shared/interfaces/src/lib/external-player-arguments.utils.ts b/libs/shared/interfaces/src/lib/external-player-arguments.utils.ts new file mode 100644 index 000000000..8c48ab5ae --- /dev/null +++ b/libs/shared/interfaces/src/lib/external-player-arguments.utils.ts @@ -0,0 +1,28 @@ +export type ExternalPlayerArgumentsInput = + | string + | readonly unknown[] + | null + | undefined; + +export function parseExternalPlayerArguments( + value: ExternalPlayerArgumentsInput +): string[] { + if (Array.isArray(value)) { + return value.map((argument) => String(argument).trim()).filter(Boolean); + } + + if (typeof value !== 'string') { + return []; + } + + return value + .split(/\r?\n/) + .map((argument) => argument.trim()) + .filter(Boolean); +} + +export function normalizeExternalPlayerArguments( + value: ExternalPlayerArgumentsInput +): string { + return parseExternalPlayerArguments(value).join('\n'); +}