fix(subtitle-sidebar): address CodeRabbit review

This commit is contained in:
2026-03-21 10:15:06 -07:00
parent ea86f4e504
commit de111dbf8d
4 changed files with 47 additions and 26 deletions

View File

@@ -4056,7 +4056,7 @@ async function extractInternalSubtitleTrackToTempFile(
videoPath: string,
track: MpvSubtitleTrackLike,
): Promise<{ path: string; cleanup: () => Promise<void> } | null> {
const ffIndex = typeof track['ff-index'] === 'number' ? track['ff-index'] : null;
const ffIndex = parseTrackId(track['ff-index']);
const codec = typeof track.codec === 'string' ? track.codec : null;
const extension = codecToExtension(codec ?? undefined);
if (ffIndex === null || extension === null) {
@@ -4066,23 +4066,31 @@ async function extractInternalSubtitleTrackToTempFile(
const tempDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'subminer-sidebar-'));
const outputPath = path.join(tempDir, `track_${ffIndex}.${extension}`);
await new Promise<void>((resolve, reject) => {
const child = spawn(ffmpegPath, buildFfmpegSubtitleExtractionArgs(videoPath, ffIndex, outputPath));
let stderr = '';
child.stderr.on('data', (chunk: Buffer) => {
stderr += chunk.toString();
try {
await new Promise<void>((resolve, reject) => {
const child = spawn(
ffmpegPath,
buildFfmpegSubtitleExtractionArgs(videoPath, ffIndex, outputPath),
);
let stderr = '';
child.stderr.on('data', (chunk: Buffer) => {
stderr += chunk.toString();
});
child.on('error', (error) => {
reject(error);
});
child.on('close', (code) => {
if (code === 0) {
resolve();
return;
}
reject(new Error(stderr.trim() || `ffmpeg exited with code ${code ?? 'unknown'}`));
});
});
child.on('error', (error) => {
reject(error);
});
child.on('close', (code) => {
if (code === 0) {
resolve();
return;
}
reject(new Error(stderr.trim() || `ffmpeg exited with code ${code ?? 'unknown'}`));
});
});
} catch (error) {
await fs.promises.rm(tempDir, { recursive: true, force: true }).catch(() => undefined);
throw error;
}
return {
path: outputPath,
@@ -4245,10 +4253,7 @@ const { registerIpcRuntimeHandlers } = composeIpcRuntimeHandlers({
};
}
if (
appState.activeParsedSubtitleCues.length > 0 &&
appState.activeParsedSubtitleSource === resolvedSource.sourceKey
) {
if (appState.activeParsedSubtitleSource === resolvedSource.sourceKey) {
return {
cues: appState.activeParsedSubtitleCues,
currentTimeSec,

View File

@@ -18,6 +18,15 @@ test('getActiveExternalSubtitleSource returns the active external subtitle path'
assert.equal(source, 'https://host/subs.ass');
});
test('getActiveExternalSubtitleSource normalizes integer-like string track ids', () => {
const source = getActiveExternalSubtitleSource(
[{ type: 'sub', id: '2', external: true, 'external-filename': ' /tmp/subs.ass ' }],
'2',
);
assert.equal(source, '/tmp/subs.ass');
});
test('getActiveExternalSubtitleSource returns null when the selected track is not external', () => {
const source = getActiveExternalSubtitleSource(
[{ type: 'sub', id: 2, external: false, 'external-filename': '/tmp/subs.ass' }],
@@ -70,6 +79,15 @@ test('buildSubtitleSidebarSourceKey uses a stable identifier for internal subtit
assert.equal(firstKey, 'internal:/media/episode01.mkv:track:3:ff:7');
});
test('buildSubtitleSidebarSourceKey normalizes integer-like string track metadata', () => {
const key = buildSubtitleSidebarSourceKey('/media/episode01.mkv', {
id: '3',
'ff-index': '7',
});
assert.equal(key, 'internal:/media/episode01.mkv:track:3:ff:7');
});
test('buildSubtitleSidebarSourceKey falls back to source path when no track metadata is available', () => {
const key = buildSubtitleSidebarSourceKey('/media/episode01.mkv', null, '/tmp/subtitle.ass');

View File

@@ -19,9 +19,8 @@ export function getActiveExternalSubtitleSource(
return null;
}
const sid =
typeof sidRaw === 'number' ? sidRaw : typeof sidRaw === 'string' ? Number(sidRaw) : null;
if (sid == null || !Number.isFinite(sid)) {
const sid = parseTrackId(sidRaw);
if (sid === null) {
return null;
}
@@ -30,7 +29,7 @@ export function getActiveExternalSubtitleSource(
return false;
}
const track = entry as Record<string, unknown>;
return track.type === 'sub' && track.id === sid;
return track.type === 'sub' && parseTrackId(track.id) === sid && track.external === true;
}) as Record<string, unknown> | undefined;
const externalFilename =

View File

@@ -726,7 +726,6 @@ body.subtitle-sidebar-embedded-open #secondarySubContainer {
position: absolute;
top: 40px;
left: 50%;
transform: translateX(-50%);
max-width: min(80%, calc(100vw - var(--subtitle-sidebar-reserved-width) - 24px));
padding: 10px 18px;
background: var(--secondary-sub-background-color, transparent);