fix: address latest review comments

This commit is contained in:
2026-02-28 22:41:43 -08:00
parent a46f90d085
commit fb948c6feb
10 changed files with 89 additions and 36 deletions

View File

@@ -71,7 +71,7 @@ auto_start = maybe
auto_start_visible_overlay = no auto_start_visible_overlay = no
auto_start_pause_until_ready = off auto_start_pause_until_ready = off
`); `);
assert.equal(parsed.autoStart, true); assert.equal(parsed.autoStart, false);
assert.equal(parsed.autoStartVisibleOverlay, false); assert.equal(parsed.autoStartVisibleOverlay, false);
assert.equal(parsed.autoStartPauseUntilReady, false); assert.equal(parsed.autoStartPauseUntilReady, false);
}); });

View File

@@ -15,7 +15,10 @@ export function getPluginConfigCandidates(): string[] {
); );
} }
export function parsePluginRuntimeConfigContent(content: string): PluginRuntimeConfig { export function parsePluginRuntimeConfigContent(
content: string,
logLevel: LogLevel = 'warn',
): PluginRuntimeConfig {
const runtimeConfig: PluginRuntimeConfig = { const runtimeConfig: PluginRuntimeConfig = {
socketPath: DEFAULT_SOCKET_PATH, socketPath: DEFAULT_SOCKET_PATH,
autoStart: true, autoStart: true,
@@ -23,11 +26,12 @@ export function parsePluginRuntimeConfigContent(content: string): PluginRuntimeC
autoStartPauseUntilReady: true, autoStartPauseUntilReady: true,
}; };
const parseBooleanValue = (value: string, fallback: boolean): boolean => { const parseBooleanValue = (key: string, value: string): boolean => {
const normalized = value.trim().toLowerCase(); const normalized = value.trim().toLowerCase();
if (['yes', 'true', '1', 'on'].includes(normalized)) return true; if (['yes', 'true', '1', 'on'].includes(normalized)) return true;
if (['no', 'false', '0', 'off'].includes(normalized)) return false; if (['no', 'false', '0', 'off'].includes(normalized)) return false;
return fallback; log('warn', logLevel, `Invalid boolean value for ${key}: "${value}". Using false.`);
return false;
}; };
for (const line of content.split(/\r?\n/)) { for (const line of content.split(/\r?\n/)) {
@@ -44,20 +48,17 @@ export function parsePluginRuntimeConfigContent(content: string): PluginRuntimeC
continue; continue;
} }
if (key === 'auto_start') { if (key === 'auto_start') {
runtimeConfig.autoStart = parseBooleanValue(value, runtimeConfig.autoStart); runtimeConfig.autoStart = parseBooleanValue('auto_start', value);
continue; continue;
} }
if (key === 'auto_start_visible_overlay') { if (key === 'auto_start_visible_overlay') {
runtimeConfig.autoStartVisibleOverlay = parseBooleanValue( runtimeConfig.autoStartVisibleOverlay = parseBooleanValue('auto_start_visible_overlay', value);
value,
runtimeConfig.autoStartVisibleOverlay,
);
continue; continue;
} }
if (key === 'auto_start_pause_until_ready') { if (key === 'auto_start_pause_until_ready') {
runtimeConfig.autoStartPauseUntilReady = parseBooleanValue( runtimeConfig.autoStartPauseUntilReady = parseBooleanValue(
'auto_start_pause_until_ready',
value, value,
runtimeConfig.autoStartPauseUntilReady,
); );
} }
} }

View File

@@ -28,6 +28,7 @@ export {
} from './startup'; } from './startup';
export { openYomitanSettingsWindow } from './yomitan-settings'; export { openYomitanSettingsWindow } from './yomitan-settings';
export { createTokenizerDepsRuntime, tokenizeSubtitle } from './tokenizer'; export { createTokenizerDepsRuntime, tokenizeSubtitle } from './tokenizer';
export { clearYomitanParserCachesForWindow } from './tokenizer/yomitan-parser-runtime';
export { syncYomitanDefaultAnkiServer } from './tokenizer/yomitan-parser-runtime'; export { syncYomitanDefaultAnkiServer } from './tokenizer/yomitan-parser-runtime';
export { createSubtitleProcessingController } from './subtitle-processing-controller'; export { createSubtitleProcessingController } from './subtitle-processing-controller';
export { createFrequencyDictionaryLookup } from './frequency-dictionary'; export { createFrequencyDictionaryLookup } from './frequency-dictionary';

View File

@@ -62,6 +62,9 @@ function clearWindowCaches(window: BrowserWindow): void {
yomitanProfileMetadataByWindow.delete(window); yomitanProfileMetadataByWindow.delete(window);
yomitanFrequencyCacheByWindow.delete(window); yomitanFrequencyCacheByWindow.delete(window);
} }
export function clearYomitanParserCachesForWindow(window: BrowserWindow): void {
clearWindowCaches(window);
}
function asPositiveInteger(value: unknown): number | null { function asPositiveInteger(value: unknown): number | null {
if (typeof value !== 'number' || !Number.isFinite(value) || value <= 0) { if (typeof value !== 'number' || !Number.isFinite(value) || value <= 0) {

View File

@@ -7,6 +7,7 @@ export interface OpenYomitanSettingsWindowOptions {
yomitanExt: Extension | null; yomitanExt: Extension | null;
getExistingWindow: () => BrowserWindow | null; getExistingWindow: () => BrowserWindow | null;
setWindow: (window: BrowserWindow | null) => void; setWindow: (window: BrowserWindow | null) => void;
onWindowClosed?: () => void;
} }
export function openYomitanSettingsWindow(options: OpenYomitanSettingsWindowOptions): void { export function openYomitanSettingsWindow(options: OpenYomitanSettingsWindowOptions): void {
@@ -81,6 +82,7 @@ export function openYomitanSettingsWindow(options: OpenYomitanSettingsWindowOpti
}, 500); }, 500);
settingsWindow.on('closed', () => { settingsWindow.on('closed', () => {
options.onWindowClosed?.();
options.setWindow(null); options.setWindow(null);
}); });
} }

View File

@@ -354,6 +354,7 @@ import {
resolveJellyfinPlaybackPlanRuntime, resolveJellyfinPlaybackPlanRuntime,
runStartupBootstrapRuntime, runStartupBootstrapRuntime,
saveSubtitlePosition as saveSubtitlePositionCore, saveSubtitlePosition as saveSubtitlePositionCore,
clearYomitanParserCachesForWindow,
syncYomitanDefaultAnkiServer as syncYomitanDefaultAnkiServerCore, syncYomitanDefaultAnkiServer as syncYomitanDefaultAnkiServerCore,
sendMpvCommandRuntime, sendMpvCommandRuntime,
setMpvSubVisibilityRuntime, setMpvSubVisibilityRuntime,
@@ -845,6 +846,7 @@ const anilistStateRuntime = createAnilistStateRuntime(buildAnilistStateRuntimeMa
const configDerivedRuntime = createConfigDerivedRuntime(buildConfigDerivedRuntimeMainDepsHandler()); const configDerivedRuntime = createConfigDerivedRuntime(buildConfigDerivedRuntimeMainDepsHandler());
const subsyncRuntime = createMainSubsyncRuntime(buildMainSubsyncRuntimeMainDepsHandler()); const subsyncRuntime = createMainSubsyncRuntime(buildMainSubsyncRuntimeMainDepsHandler());
let autoPlayReadySignalMediaPath: string | null = null; let autoPlayReadySignalMediaPath: string | null = null;
let autoPlayReadySignalGeneration = 0;
function maybeSignalPluginAutoplayReady(payload: SubtitleData): void { function maybeSignalPluginAutoplayReady(payload: SubtitleData): void {
if (!payload.text.trim()) { if (!payload.text.trim()) {
@@ -858,8 +860,32 @@ function maybeSignalPluginAutoplayReady(payload: SubtitleData): void {
return; return;
} }
autoPlayReadySignalMediaPath = mediaPath; autoPlayReadySignalMediaPath = mediaPath;
const playbackGeneration = ++autoPlayReadySignalGeneration;
logger.debug(`[autoplay-ready] signaling mpv for media: ${mediaPath}`); logger.debug(`[autoplay-ready] signaling mpv for media: ${mediaPath}`);
sendMpvCommandRuntime(appState.mpvClient, ['script-message', 'subminer-autoplay-ready']); sendMpvCommandRuntime(appState.mpvClient, ['script-message', 'subminer-autoplay-ready']);
const isPlaybackPaused = async (client: {
requestProperty: (property: string) => Promise<unknown>;
}): Promise<boolean> => {
try {
const pauseProperty = await client.requestProperty('pause');
if (typeof pauseProperty === 'boolean') {
return pauseProperty;
}
if (typeof pauseProperty === 'string') {
return pauseProperty.toLowerCase() !== 'no' && pauseProperty !== '0';
}
if (typeof pauseProperty === 'number') {
return pauseProperty !== 0;
}
logger.debug(`[autoplay-ready] unrecognized pause property for media ${mediaPath}: ${String(pauseProperty)}`);
} catch (error) {
logger.debug(
`[autoplay-ready] failed to read pause property for media ${mediaPath}: ${(error as Error).message}`,
);
}
return true;
};
// Fallback: unpause directly in case plugin readiness handler is unavailable/outdated. // Fallback: unpause directly in case plugin readiness handler is unavailable/outdated.
void (async () => { void (async () => {
const mpvClient = appState.mpvClient; const mpvClient = appState.mpvClient;
@@ -868,20 +894,8 @@ function maybeSignalPluginAutoplayReady(payload: SubtitleData): void {
return; return;
} }
let shouldUnpause = appState.playbackPaused !== false; const shouldUnpause = await isPlaybackPaused(mpvClient);
try { logger.debug(`[autoplay-ready] mpv paused before fallback for ${mediaPath}: ${shouldUnpause}`);
const pauseProperty = await mpvClient.requestProperty('pause');
if (typeof pauseProperty === 'boolean') {
shouldUnpause = pauseProperty;
} else if (typeof pauseProperty === 'string') {
shouldUnpause = pauseProperty.toLowerCase() !== 'no' && pauseProperty !== '0';
}
logger.debug(`[autoplay-ready] mpv pause property before fallback: ${String(pauseProperty)}`);
} catch (error) {
logger.debug(
`[autoplay-ready] failed to read pause property before fallback: ${(error as Error).message}`,
);
}
if (!shouldUnpause) { if (!shouldUnpause) {
logger.debug('[autoplay-ready] mpv already playing; no fallback unpause needed'); logger.debug('[autoplay-ready] mpv already playing; no fallback unpause needed');
@@ -890,10 +904,25 @@ function maybeSignalPluginAutoplayReady(payload: SubtitleData): void {
mpvClient.send({ command: ['set_property', 'pause', false] }); mpvClient.send({ command: ['set_property', 'pause', false] });
setTimeout(() => { setTimeout(() => {
const followupClient = appState.mpvClient; void (async () => {
if (followupClient?.connected) { if (
followupClient.send({ command: ['set_property', 'pause', false] }); autoPlayReadySignalMediaPath !== mediaPath ||
playbackGeneration !== autoPlayReadySignalGeneration
) {
return;
} }
const followupClient = appState.mpvClient;
if (!followupClient?.connected) {
return;
}
const shouldUnpauseFollowup = await isPlaybackPaused(followupClient);
if (!shouldUnpauseFollowup) {
return;
}
followupClient.send({ command: ['set_property', 'pause', false] });
})();
}, 500); }, 500);
logger.debug('[autoplay-ready] issued direct mpv unpause fallback'); logger.debug('[autoplay-ready] issued direct mpv unpause fallback');
})(); })();
@@ -3177,6 +3206,11 @@ const { openYomitanSettings: openYomitanSettingsHandler } = createYomitanSetting
yomitanExt: yomitanExt as Extension, yomitanExt: yomitanExt as Extension,
getExistingWindow: () => getExistingWindow() as BrowserWindow | null, getExistingWindow: () => getExistingWindow() as BrowserWindow | null,
setWindow: (window) => setWindow(window as BrowserWindow | null), setWindow: (window) => setWindow(window as BrowserWindow | null),
onWindowClosed: () => {
if (appState.yomitanParserWindow) {
clearYomitanParserCachesForWindow(appState.yomitanParserWindow);
}
},
}); });
}, },
getExistingWindow: () => appState.yomitanSettingsWindow, getExistingWindow: () => appState.yomitanSettingsWindow,

View File

@@ -66,6 +66,7 @@ export function createBuildOpenYomitanSettingsMainDepsHandler<TYomitanExt, TWind
yomitanExt: TYomitanExt; yomitanExt: TYomitanExt;
getExistingWindow: () => TWindow | null; getExistingWindow: () => TWindow | null;
setWindow: (window: TWindow | null) => void; setWindow: (window: TWindow | null) => void;
onWindowClosed?: () => void;
}) => void; }) => void;
getExistingWindow: () => TWindow | null; getExistingWindow: () => TWindow | null;
setWindow: (window: TWindow | null) => void; setWindow: (window: TWindow | null) => void;
@@ -78,6 +79,7 @@ export function createBuildOpenYomitanSettingsMainDepsHandler<TYomitanExt, TWind
yomitanExt: TYomitanExt; yomitanExt: TYomitanExt;
getExistingWindow: () => TWindow | null; getExistingWindow: () => TWindow | null;
setWindow: (window: TWindow | null) => void; setWindow: (window: TWindow | null) => void;
onWindowClosed?: () => void;
}) => deps.openYomitanSettingsWindow(params), }) => deps.openYomitanSettingsWindow(params),
getExistingWindow: () => deps.getExistingWindow(), getExistingWindow: () => deps.getExistingWindow(),
setWindow: (window: TWindow | null) => deps.setWindow(window), setWindow: (window: TWindow | null) => deps.setWindow(window),

View File

@@ -7,6 +7,7 @@ export function createOpenYomitanSettingsHandler(deps: {
yomitanExt: YomitanExtensionLike; yomitanExt: YomitanExtensionLike;
getExistingWindow: () => BrowserWindowLike | null; getExistingWindow: () => BrowserWindowLike | null;
setWindow: (window: BrowserWindowLike | null) => void; setWindow: (window: BrowserWindowLike | null) => void;
onWindowClosed?: () => void;
}) => void; }) => void;
getExistingWindow: () => BrowserWindowLike | null; getExistingWindow: () => BrowserWindowLike | null;
setWindow: (window: BrowserWindowLike | null) => void; setWindow: (window: BrowserWindowLike | null) => void;

View File

@@ -85,7 +85,7 @@ test('auto-pause on subtitle hover pauses on enter and resumes on leave when ena
}); });
await handlers.handleMouseEnter(); await handlers.handleMouseEnter();
handlers.handleMouseLeave(); await handlers.handleMouseLeave();
assert.deepEqual(mpvCommands, [ assert.deepEqual(mpvCommands, [
['set_property', 'pause', 'yes'], ['set_property', 'pause', 'yes'],
@@ -93,9 +93,10 @@ test('auto-pause on subtitle hover pauses on enter and resumes on leave when ena
]); ]);
}); });
test('auto-pause on subtitle hover does not unpause when playback was already paused', async () => { test('auto-pause on subtitle hover does not unpause when playback becomes paused on leave', async () => {
const ctx = createMouseTestContext(); const ctx = createMouseTestContext();
const mpvCommands: Array<(string | number)[]> = []; const mpvCommands: Array<(string | number)[]> = [];
const playbackPausedStates = [false, true];
const handlers = createMouseHandlers(ctx as never, { const handlers = createMouseHandlers(ctx as never, {
modalStateReader: { modalStateReader: {
@@ -106,16 +107,16 @@ test('auto-pause on subtitle hover does not unpause when playback was already pa
getCurrentYPercent: () => 10, getCurrentYPercent: () => 10,
persistSubtitlePositionPatch: () => {}, persistSubtitlePositionPatch: () => {},
getSubtitleHoverAutoPauseEnabled: () => true, getSubtitleHoverAutoPauseEnabled: () => true,
getPlaybackPaused: async () => true, getPlaybackPaused: async () => playbackPausedStates.shift() ?? true,
sendMpvCommand: (command) => { sendMpvCommand: (command) => {
mpvCommands.push(command); mpvCommands.push(command);
}, },
}); });
await handlers.handleMouseEnter(); await handlers.handleMouseEnter();
handlers.handleMouseLeave(); await handlers.handleMouseLeave();
assert.deepEqual(mpvCommands, []); assert.deepEqual(mpvCommands, [['set_property', 'pause', 'yes']]);
}); });
test('auto-pause on subtitle hover is skipped when disabled in config', async () => { test('auto-pause on subtitle hover is skipped when disabled in config', async () => {
@@ -138,7 +139,7 @@ test('auto-pause on subtitle hover is skipped when disabled in config', async ()
}); });
await handlers.handleMouseEnter(); await handlers.handleMouseEnter();
handlers.handleMouseLeave(); await handlers.handleMouseLeave();
assert.deepEqual(mpvCommands, []); assert.deepEqual(mpvCommands, []);
}); });
@@ -164,7 +165,7 @@ test('pending hover pause check is ignored when mouse leaves before pause state
}); });
const enterPromise = handlers.handleMouseEnter(); const enterPromise = handlers.handleMouseEnter();
handlers.handleMouseLeave(); await handlers.handleMouseLeave();
deferred.resolve(false); deferred.resolve(false);
await enterPromise; await enterPromise;

View File

@@ -76,12 +76,20 @@ export function createMouseHandlers(
pausedBySubtitleHover = true; pausedBySubtitleHover = true;
} }
function handleMouseLeave(): void { async function handleMouseLeave(): Promise<void> {
ctx.state.isOverSubtitle = false; ctx.state.isOverSubtitle = false;
hoverPauseRequestId += 1; hoverPauseRequestId += 1;
if (pausedBySubtitleHover) { if (pausedBySubtitleHover) {
options.sendMpvCommand(['set_property', 'pause', 'no']);
pausedBySubtitleHover = false; pausedBySubtitleHover = false;
try {
const isPaused = await options.getPlaybackPaused();
if (isPaused !== false) {
return;
}
} catch {
return;
}
options.sendMpvCommand(['set_property', 'pause', 'no']);
} }
if (yomitanPopupVisible) return; if (yomitanPopupVisible) return;
disablePopupInteractionIfIdle(); disablePopupInteractionIfIdle();