mirror of
https://github.com/ksyasuda/SubMiner.git
synced 2026-05-26 00:55:16 -07:00
fix: address follow-up review feedback
This commit is contained in:
@@ -1842,6 +1842,7 @@ test('runtime options registry is centralized', () => {
|
||||
const ids = RUNTIME_OPTION_REGISTRY.map((entry) => entry.id);
|
||||
assert.deepEqual(ids, [
|
||||
'anki.autoUpdateNewCards',
|
||||
'subtitle.annotation.knownWords.highlightEnabled',
|
||||
'subtitle.annotation.nPlusOne',
|
||||
'subtitle.annotation.jlpt',
|
||||
'subtitle.annotation.frequency',
|
||||
|
||||
@@ -20,9 +20,9 @@ export function buildRuntimeOptionRegistry(
|
||||
}),
|
||||
},
|
||||
{
|
||||
id: 'subtitle.annotation.nPlusOne',
|
||||
id: 'subtitle.annotation.knownWords.highlightEnabled',
|
||||
path: 'ankiConnect.knownWords.highlightEnabled',
|
||||
label: 'N+1 Annotation',
|
||||
label: 'Known Word Annotation',
|
||||
scope: 'subtitle',
|
||||
valueType: 'boolean',
|
||||
allowedValues: [true, false],
|
||||
@@ -35,6 +35,22 @@ export function buildRuntimeOptionRegistry(
|
||||
},
|
||||
}),
|
||||
},
|
||||
{
|
||||
id: 'subtitle.annotation.nPlusOne',
|
||||
path: 'ankiConnect.nPlusOne.enabled',
|
||||
label: 'N+1 Annotation',
|
||||
scope: 'subtitle',
|
||||
valueType: 'boolean',
|
||||
allowedValues: [true, false],
|
||||
defaultValue: defaultConfig.ankiConnect.nPlusOne.enabled,
|
||||
requiresRestart: false,
|
||||
formatValueForOsd: (value) => (value === true ? 'On' : 'Off'),
|
||||
toAnkiPatch: (value) => ({
|
||||
nPlusOne: {
|
||||
enabled: value === true,
|
||||
},
|
||||
}),
|
||||
},
|
||||
{
|
||||
id: 'subtitle.annotation.jlpt',
|
||||
path: 'subtitleStyle.enableJlpt',
|
||||
|
||||
@@ -89,7 +89,7 @@ const JSON_OBJECT_FIELDS = new Set([
|
||||
|
||||
const SECRET_PATHS = new Set(['ai.apiKey', 'jimaku.apiKey', 'anilist.accessToken']);
|
||||
|
||||
const COLOR_SUFFIXES = new Set(['Color', 'color', 'backgroundColor', 'singleColor', 'nPlusOne']);
|
||||
const COLOR_SUFFIXES = new Set(['Color', 'color', 'backgroundColor', 'singleColor']);
|
||||
const SUBTITLE_CSS_MANAGED_CONFIG_PATHS = new Set([
|
||||
...getSubtitleCssManagedConfigPaths('primary'),
|
||||
...getSubtitleCssManagedConfigPaths('secondary'),
|
||||
@@ -111,6 +111,9 @@ const CATEGORY_ORDER: ConfigSettingsCategory[] = [
|
||||
const SECTION_ORDER = new Map<string, number>(
|
||||
[
|
||||
'Annotation Display',
|
||||
'Known Words',
|
||||
'N+1',
|
||||
'Frequency Highlighting',
|
||||
'Primary Subtitle Appearance',
|
||||
'Secondary Subtitle Appearance',
|
||||
'Subtitle Sidebar Appearance',
|
||||
|
||||
+17
-5
@@ -665,14 +665,18 @@ const texthookerService = new Texthooker(() => {
|
||||
const characterDictionaryEnabled =
|
||||
config.anilist.characterDictionary.enabled &&
|
||||
yomitanProfilePolicy.isCharacterDictionaryEnabled();
|
||||
const knownAndNPlusOneEnabled = getRuntimeBooleanOption(
|
||||
const knownWordColoringEnabled = getRuntimeBooleanOption(
|
||||
'subtitle.annotation.knownWords.highlightEnabled',
|
||||
config.ankiConnect.knownWords.highlightEnabled,
|
||||
);
|
||||
const nPlusOneColoringEnabled = getRuntimeBooleanOption(
|
||||
'subtitle.annotation.nPlusOne',
|
||||
config.ankiConnect.nPlusOne.enabled,
|
||||
);
|
||||
|
||||
return {
|
||||
enableKnownWordColoring: knownAndNPlusOneEnabled,
|
||||
enableNPlusOneColoring: knownAndNPlusOneEnabled,
|
||||
enableKnownWordColoring: knownWordColoringEnabled,
|
||||
enableNPlusOneColoring: nPlusOneColoringEnabled,
|
||||
enableNameMatchColoring: config.subtitleStyle.nameMatchEnabled && characterDictionaryEnabled,
|
||||
enableFrequencyColoring: getRuntimeBooleanOption(
|
||||
'subtitle.annotation.frequency',
|
||||
@@ -2578,7 +2582,11 @@ function getResolvedConfig() {
|
||||
}
|
||||
|
||||
function getRuntimeBooleanOption(
|
||||
id: 'subtitle.annotation.nPlusOne' | 'subtitle.annotation.jlpt' | 'subtitle.annotation.frequency',
|
||||
id:
|
||||
| 'subtitle.annotation.knownWords.highlightEnabled'
|
||||
| 'subtitle.annotation.nPlusOne'
|
||||
| 'subtitle.annotation.jlpt'
|
||||
| 'subtitle.annotation.frequency',
|
||||
fallback: boolean,
|
||||
): boolean {
|
||||
const value = appState.runtimeOptionsManager?.getOptionValue(id);
|
||||
@@ -2587,6 +2595,10 @@ function getRuntimeBooleanOption(
|
||||
|
||||
function shouldInitializeMecabForAnnotations(): boolean {
|
||||
const config = getResolvedConfig();
|
||||
const knownWordsEnabled = getRuntimeBooleanOption(
|
||||
'subtitle.annotation.knownWords.highlightEnabled',
|
||||
config.ankiConnect.knownWords.highlightEnabled,
|
||||
);
|
||||
const nPlusOneEnabled = getRuntimeBooleanOption(
|
||||
'subtitle.annotation.nPlusOne',
|
||||
config.ankiConnect.nPlusOne.enabled,
|
||||
@@ -2599,7 +2611,7 @@ function shouldInitializeMecabForAnnotations(): boolean {
|
||||
'subtitle.annotation.frequency',
|
||||
config.subtitleStyle.frequencyDictionary.enabled,
|
||||
);
|
||||
return nPlusOneEnabled || jlptEnabled || frequencyEnabled;
|
||||
return knownWordsEnabled || nPlusOneEnabled || jlptEnabled || frequencyEnabled;
|
||||
}
|
||||
|
||||
const {
|
||||
|
||||
@@ -521,7 +521,7 @@ test('mpv input forwarding installs local key handling when session binding IPC
|
||||
testGlobals.setGetSessionBindings(() => new Promise<CompiledSessionBinding[]>(() => {}));
|
||||
const setupResult = await Promise.race([
|
||||
handlers.setupMpvInputForwarding().then(() => 'resolved'),
|
||||
wait(25).then(() => 'pending'),
|
||||
wait(75).then(() => 'pending'),
|
||||
]);
|
||||
|
||||
assert.equal(setupResult, 'resolved');
|
||||
@@ -533,6 +533,35 @@ test('mpv input forwarding installs local key handling when session binding IPC
|
||||
}
|
||||
});
|
||||
|
||||
test('mpv input forwarding waits for session bindings before resolving setup', async () => {
|
||||
const { handlers, testGlobals } = createKeyboardHandlerHarness();
|
||||
|
||||
try {
|
||||
testGlobals.setGetSessionBindings(async () => {
|
||||
await wait(20);
|
||||
return [
|
||||
{
|
||||
sourcePath: 'keybindings[0].key',
|
||||
originalKey: 'KeyH',
|
||||
key: { code: 'KeyH', modifiers: [] },
|
||||
actionType: 'mpv-command',
|
||||
command: ['cycle', 'pause'],
|
||||
},
|
||||
] as CompiledSessionBinding[];
|
||||
});
|
||||
|
||||
await handlers.setupMpvInputForwarding();
|
||||
|
||||
assert.deepEqual(handlers.getSessionHelpOpeningInfo(), {
|
||||
bindingKey: 'KeyK',
|
||||
fallbackUsed: true,
|
||||
fallbackUnavailable: false,
|
||||
});
|
||||
} finally {
|
||||
testGlobals.restore();
|
||||
}
|
||||
});
|
||||
|
||||
test('mpv input forwarding retries a transient keyboard config IPC failure', async () => {
|
||||
const { handlers, testGlobals } = createKeyboardHandlerHarness();
|
||||
let calls = 0;
|
||||
|
||||
@@ -35,6 +35,7 @@ export function createKeyboardHandlers(
|
||||
) {
|
||||
// Timeout for the modal chord capture window (e.g. Y followed by H/K).
|
||||
const CHORD_TIMEOUT_MS = 1000;
|
||||
const MPV_INPUT_FORWARDING_CONFIG_LOAD_TIMEOUT_MS = 50;
|
||||
const KEYBOARD_SELECTED_WORD_CLASS = 'keyboard-selected';
|
||||
let pendingSelectionAnchorAfterSubtitleSeek: 'start' | 'end' | null = null;
|
||||
let pendingLookupRefreshAfterSubtitleSeek = false;
|
||||
@@ -975,26 +976,21 @@ export function createKeyboardHandlers(
|
||||
installMpvInputForwardingListeners();
|
||||
syncKeyboardTokenSelection();
|
||||
|
||||
let configLoadSettled = false;
|
||||
let configLoadError: unknown = null;
|
||||
const configLoad = loadMpvInputForwardingConfigWithRetry().then(
|
||||
() => {
|
||||
configLoadSettled = true;
|
||||
},
|
||||
() => {},
|
||||
(error) => {
|
||||
configLoadSettled = true;
|
||||
configLoadError = error;
|
||||
console.error('Failed to load overlay keyboard configuration.', error);
|
||||
},
|
||||
);
|
||||
|
||||
await new Promise<void>((resolve) => {
|
||||
setTimeout(resolve, 0);
|
||||
});
|
||||
if (!configLoadSettled) {
|
||||
void configLoad;
|
||||
return;
|
||||
}
|
||||
await Promise.race([
|
||||
configLoad,
|
||||
new Promise<void>((resolve) => {
|
||||
setTimeout(resolve, MPV_INPUT_FORWARDING_CONFIG_LOAD_TIMEOUT_MS);
|
||||
}),
|
||||
]);
|
||||
if (configLoadError) {
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -138,6 +138,14 @@ test('applySidebarCssDeclarations clears declarations removed by config reload',
|
||||
assert.equal(style.color, '#ffffff');
|
||||
assert.equal(style.backgroundColor, '');
|
||||
assert.deepEqual(removed, ['background-color']);
|
||||
|
||||
applySidebarCssDeclarations(target, {
|
||||
color: '',
|
||||
'background-color': '',
|
||||
});
|
||||
|
||||
assert.equal(style.color, '');
|
||||
assert.deepEqual(removed, ['background-color', 'background-color']);
|
||||
});
|
||||
|
||||
test('subtitle sidebar modal opens from snapshot and clicking cue seeks playback', async () => {
|
||||
|
||||
@@ -77,7 +77,14 @@ export function applySidebarCssDeclarations(
|
||||
|
||||
for (const [property, rawValue] of Object.entries(declarations)) {
|
||||
const value = rawValue.trim();
|
||||
if (value.length === 0) continue;
|
||||
if (value.length === 0) {
|
||||
if (property.includes('-')) {
|
||||
targetStyle.removeProperty(property);
|
||||
} else {
|
||||
styleTarget[property] = '';
|
||||
}
|
||||
continue;
|
||||
}
|
||||
if (property.includes('-')) {
|
||||
targetStyle.setProperty(property, value);
|
||||
} else {
|
||||
|
||||
@@ -3,6 +3,7 @@ import fs from 'node:fs';
|
||||
import path from 'node:path';
|
||||
import test from 'node:test';
|
||||
|
||||
import { DEFAULT_CONFIG } from './config';
|
||||
import { RuntimeOptionsManager } from './runtime-options';
|
||||
|
||||
test('SM-012 runtime options path does not use JSON serialize-clone helpers', () => {
|
||||
@@ -59,3 +60,25 @@ test('RuntimeOptionsManager returns detached effective Anki config copies', () =
|
||||
assert.deepEqual(baseConfig.tags, ['SubMiner']);
|
||||
assert.equal(baseConfig.behavior.autoUpdateNewCards, true);
|
||||
});
|
||||
|
||||
test('RuntimeOptionsManager keeps known-word and n+1 annotation toggles separate', () => {
|
||||
const baseConfig = structuredClone(DEFAULT_CONFIG.ankiConnect);
|
||||
const patches: unknown[] = [];
|
||||
const manager = new RuntimeOptionsManager(() => structuredClone(baseConfig), {
|
||||
applyAnkiPatch: (patch) => {
|
||||
patches.push(patch);
|
||||
},
|
||||
onOptionsChanged: () => undefined,
|
||||
});
|
||||
|
||||
assert.equal(
|
||||
manager.setOptionValue('subtitle.annotation.knownWords.highlightEnabled', true).ok,
|
||||
true,
|
||||
);
|
||||
assert.equal(manager.setOptionValue('subtitle.annotation.nPlusOne', true).ok, true);
|
||||
|
||||
const effective = manager.getEffectiveAnkiConnectConfig();
|
||||
assert.equal(effective.knownWords?.highlightEnabled, true);
|
||||
assert.equal(effective.nPlusOne?.enabled, true);
|
||||
assert.deepEqual(patches, []);
|
||||
});
|
||||
|
||||
@@ -140,8 +140,8 @@ export function renderMpvKeybindingsInput(
|
||||
removeButton.type = 'button';
|
||||
removeButton.textContent = 'Remove';
|
||||
removeButton.addEventListener('click', () => {
|
||||
rows.splice(i, 1);
|
||||
applyMpvRows(context, field, rows);
|
||||
const nextRows = rows.filter((_, index) => index !== i);
|
||||
applyMpvRows(context, field, nextRows);
|
||||
requestRender();
|
||||
});
|
||||
item.append(keyButton, command, removeButton);
|
||||
|
||||
@@ -87,6 +87,27 @@ test('filterSettingsFields normalizes punctuation in query terms', () => {
|
||||
);
|
||||
});
|
||||
|
||||
test('filterSettingsFields preserves non-Latin query terms', () => {
|
||||
const japaneseFields: ConfigSettingsField[] = [
|
||||
{
|
||||
id: 'subtitleStyle.japaneseFontFamily',
|
||||
label: '日本語フォント',
|
||||
description: '字幕の表示に使う書体。',
|
||||
configPath: 'subtitleStyle.japaneseFontFamily',
|
||||
category: 'appearance',
|
||||
section: 'Primary Subtitle Appearance',
|
||||
control: 'text',
|
||||
defaultValue: '',
|
||||
restartBehavior: 'hot-reload',
|
||||
},
|
||||
];
|
||||
|
||||
assert.deepEqual(
|
||||
filterSettingsFields(japaneseFields, { query: '日本語' }).map((field) => field.configPath),
|
||||
['subtitleStyle.japaneseFontFamily'],
|
||||
);
|
||||
});
|
||||
|
||||
test('settings draft tracks dirty set and emits save operations', () => {
|
||||
const draft = createSettingsDraft({
|
||||
'subtitleStyle.autoPauseVideoOnHover': true,
|
||||
|
||||
@@ -17,7 +17,7 @@ export interface SettingsDraft {
|
||||
}
|
||||
|
||||
function normalizeQuery(query: string | undefined): string {
|
||||
return (query ?? '').trim().toLowerCase();
|
||||
return (query ?? '').trim().toLocaleLowerCase();
|
||||
}
|
||||
|
||||
function searchableText(parts: Array<string | undefined>): string {
|
||||
@@ -25,8 +25,8 @@ function searchableText(parts: Array<string | undefined>): string {
|
||||
.filter(Boolean)
|
||||
.join(' ')
|
||||
.replace(/([a-z0-9])([A-Z])/g, '$1 $2')
|
||||
.replace(/[^a-zA-Z0-9]+/g, ' ')
|
||||
.toLowerCase();
|
||||
.replace(/[^\p{L}\p{N}]+/gu, ' ')
|
||||
.toLocaleLowerCase();
|
||||
}
|
||||
|
||||
function valuesEqual(a: unknown, b: unknown): boolean {
|
||||
|
||||
@@ -48,6 +48,7 @@ const SESSION_ACTION_IDS: SessionActionId[] = [
|
||||
|
||||
const RUNTIME_OPTION_IDS: RuntimeOptionId[] = [
|
||||
'anki.autoUpdateNewCards',
|
||||
'subtitle.annotation.knownWords.highlightEnabled',
|
||||
'subtitle.annotation.nPlusOne',
|
||||
'subtitle.annotation.jlpt',
|
||||
'subtitle.annotation.frequency',
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
export type RuntimeOptionId =
|
||||
| 'anki.autoUpdateNewCards'
|
||||
| 'subtitle.annotation.knownWords.highlightEnabled'
|
||||
| 'subtitle.annotation.nPlusOne'
|
||||
| 'subtitle.annotation.jlpt'
|
||||
| 'subtitle.annotation.frequency'
|
||||
|
||||
Reference in New Issue
Block a user