feat(captions): export subtitles as separate SRT/VTT file alongside video#599
Open
mvanhorn wants to merge 2 commits into
Open
feat(captions): export subtitles as separate SRT/VTT file alongside video#599mvanhorn wants to merge 2 commits into
mvanhorn wants to merge 2 commits into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds end-to-end subtitle export: typed renderer API, cue normalization and SRT/VTT serialization, Electron save-dialog file write with approval, unit tests, IPC/preload/register wiring, and VideoEditor UI export controls. ChangesSubtitle Export Feature
Sequence Diagram(s)sequenceDiagram
participant VideoEditor
participant Preload as electronAPI
participant IPCMain
participant Handler as exportSubtitleFile
participant Dialog as dialog.showSaveDialog
participant FS as fs.writeFile
participant Approve as approveUserPath
VideoEditor->>Preload: invoke exportSubtitleFile({format,cues,fileName})
Preload->>IPCMain: ipcRenderer.invoke('export-subtitle-file', options)
IPCMain->>Handler: receive options
Handler->>Handler: normalize cues & serialize to SRT/VTT text
Handler->>Dialog: showSaveDialog(with format filters)
Dialog-->>Handler: path or canceled
alt user canceled
Handler-->>IPCMain: {success:false, canceled:true}
else path selected
Handler->>FS: writeFile(path, content)
FS-->>Handler: success or error
Handler->>Approve: approveUserPath(path)
Approve-->>Handler: approved
Handler-->>IPCMain: {success:true, path}
end
IPCMain-->>Preload: resolved result
Preload-->>VideoEditor: result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@electron/ipc/captions/exportSubtitleFile.ts`:
- Around line 44-55: The validation currently checks endMs <= startMs before
clamping, allowing negative times like startMs=-500,endMs=-100 to pass and then
serialize to zero-duration cues; update the logic in the caption normalization
loop (the block that currently logs and returns, and the push into
normalizedCues) to clamp startMs and endMs to a minimum of 0 (and optionally to
fileDuration if available) before performing the duration check, then apply the
existing text.trim() and endMs <= startMs validation and only push into
normalizedCues after clamping and validation.
In `@src/components/video-editor/VideoEditor.tsx`:
- Line 366: The regex used to compute safeBaseName
(sourceName.replace(/[\x00-\x1f<>:"\/\\|?*]+/g, "-")) triggers Biome lint rules
for control-character and useless-escape usage; replace it with a Unicode-aware
character-class that uses the control category and removes unnecessary escapes,
e.g. use sourceName.replace(/[\p{Cc}<>:"\\|?*]+/gu, "-") in the VideoEditor.tsx
code where safeBaseName is computed so it matches all control characters via
\p{Cc}, adds the 'u' flag, and no longer contains useless escapes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3f192877-9227-4908-95ff-4c9e5e83eb99
📒 Files selected for processing (7)
electron/electron-env.d.tselectron/ipc/captions/exportSubtitleFile.test.tselectron/ipc/captions/exportSubtitleFile.tselectron/ipc/register/captions.tselectron/preload.tssrc/components/video-editor/VideoEditor.tsxsrc/lib/exporter/types.ts
| ?.split(/[\\/]/) | ||
| .pop() | ||
| ?.replace(/\.[^.]+$/, "") ?? "captions"; | ||
| const safeBaseName = sourceName.replace(/[\x00-\x1f<>:"\/\\|?*]+/g, "-").trim(); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="src/components/video-editor/VideoEditor.tsx"
python - <<'PY'
from pathlib import Path
p = Path("src/components/video-editor/VideoEditor.tsx")
line = p.read_text(encoding="utf-8").splitlines()[365] # Line 366 (1-based)
print("Line 366 repr:", repr(line))
print("Contains raw control chars:", any(ord(ch) < 32 for ch in line))
PY
rg -n 'sourceName\.replace\(' "$FILE"
# Optional: run if Biome is available in your environment
if command -v biome >/dev/null 2>&1; then
biome check "$FILE"
elif command -v npx >/dev/null 2>&1; then
npx --yes `@biomejs/biome` check "$FILE"
fiRepository: webadderallorg/Recordly
Length of output: 2460
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ORIG="src/components/video-editor/VideoEditor.tsx"
TMPDIR="$(mktemp -d)"
TMP1="$TMPDIR/VideoEditor.tsx"
# Candidate regex strings (the whole .replace(...) call argument)
# 1) As proposed in the review comment (unescaped / inside the class; \u range)
C1='sourceName.replace(/[<>:"/\\|?*\u0000-\u001F]+/g, "-").trim();'
# 2) Same but keep backslash order explicit (still noControl chars via \u)
C2='sourceName.replace(/[\u0000-\u001F<>:"/\\|?*]+/g, "-").trim();'
# 3) Try using Unicode property escape for control chars (may satisfy rule)
C3='sourceName.replace(/[<>:"/\\|?*\p{Cc}]+/gu, "-").trim();'
# 4) Fallback: escape / inside the class (should be harmless even if Biome dislikes it)
C4='sourceName.replace(/[<>:"\/\\|?*\u0000-\u001F]+/g, "-").trim();'
# Extract current line 366 and ensure it matches what we expect
python3 - <<'PY'
from pathlib import Path
p = Path("src/components/video-editor/VideoEditor.tsx")
line = p.read_text(encoding="utf-8").splitlines()[365]
print("Original line 366:", line)
PY
for i in 1 2 3 4; do
cp "$ORIG" "$TMP1"
# Replace the entire line containing safeBaseName assignment.
# Use a conservative match for the prefix to avoid accidental changes elsewhere.
python3 - <<PY
from pathlib import Path
import re
p = Path("$TMP1")
lines = p.read_text(encoding="utf-8").splitlines()
idx = 365 # line 366 (1-based)
old = lines[idx]
new = ${!i}
# Above indirection doesn't work; set via explicit mapping below
PY
doneRepository: webadderallorg/Recordly
Length of output: 227
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ORIG="src/components/video-editor/VideoEditor.tsx"
# Ensure biome is available
if command -v biome >/dev/null 2>&1; then
BIOME=(biome)
elif command -v npx >/dev/null 2>&1; then
BIOME=(npx --yes `@biomejs/biome`)
else
echo "Neither biome nor npx/@biomejs/biome is available in this environment."
exit 1
fi
python3 - <<'PY'
from pathlib import Path
p = Path("src/components/video-editor/VideoEditor.tsx")
lines = p.read_text(encoding="utf-8").splitlines()
print(lines[365])
PY
variants=(
'const safeBaseName = sourceName.replace(/[<>:"\/\\|?*\u0000-\u001F]+/g, "-").trim();'
'const safeBaseName = sourceName.replace(/[\u0000-\u001F<>:"\/\\|?*]+/g, "-").trim();'
'const safeBaseName = sourceName.replace(/[<>:"\/\\|?*\p{Cc}]+/gu, "-").trim();'
'const safeBaseName = sourceName.replace(/[<>:"/\\|?*\u0000-\u001F]+/g, "-").trim();'
)
TMPDIR="$(mktemp -d)"
trap 'rm -rf "$TMPDIR"' EXIT
idx=365 # 0-based => line 366 (1-based)
for i in "${!variants[@]}"; do
tmp="$TMPDIR/VideoEditor.$i.tsx"
cp "$ORIG" "$tmp"
# Replace exactly the assignment line, preserving leading whitespace.
python3 - "$tmp" "$idx" "${variants[$i]}" <<'PY'
import sys
from pathlib import Path
tmp = Path(sys.argv[1])
idx = int(sys.argv[2])
new_stmt = sys.argv[3]
lines = tmp.read_text(encoding="utf-8").splitlines(True) # keep endlines
old_line = lines[idx]
leading = old_line[:len(old_line) - len(old_line.lstrip(' \t'))]
lines[idx] = leading + new_stmt + ("\n" if old_line.endswith("\n") else "")
tmp.write_text(''.join(lines), encoding="utf-8")
PY
echo "===== Variant $i ====="
"${BIOME[@]}" check "$tmp" || true
doneRepository: webadderallorg/Recordly
Length of output: 6967
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ORIG="src/components/video-editor/VideoEditor.tsx"
if command -v biome >/dev/null 2>&1; then
BIOME=(biome)
elif command -v npx >/dev/null 2>&1; then
BIOME=(npx --yes `@biomejs/biome`)
else
echo "biome not available"
exit 1
fi
TMPDIR="$(mktemp -d)"
trap 'rm -rf "$TMPDIR"' EXIT
tmp="$TMPDIR/VideoEditor.fixed.tsx"
cp "$ORIG" "$tmp"
new_stmt='const safeBaseName = sourceName.replace(/[<>:"/\\|?*\p{Cc}]+/gu, "-").trim();'
python3 - "$tmp" '365' "$new_stmt" <<'PY'
import sys
from pathlib import Path
p = Path(sys.argv[1])
idx = int(sys.argv[2])
new_stmt = sys.argv[3]
lines = p.read_text(encoding="utf-8").splitlines(True)
old_line = lines[idx]
leading = old_line[:len(old_line) - len(old_line.lstrip(' \t'))]
lines[idx] = leading + new_stmt + ("\n" if old_line.endswith("\n") else "")
p.write_text(''.join(lines), encoding="utf-8")
PY
"${BIOME[@]}" check "$tmp" || trueRepository: webadderallorg/Recordly
Length of output: 110
Fix the Biome-lint-blocking regex used to sanitize safeBaseName (Line 366)
Biome reports lint/suspicious/noControlCharactersInRegex and lint/complexity/noUselessEscapeInRegex on the current pattern. Update it to use Unicode control-category escaping:
Proposed diff
- const safeBaseName = sourceName.replace(/[\x00-\x1f<>:"\/\\|?*]+/g, "-").trim();
+ const safeBaseName = sourceName.replace(/[<>:"/\\|?*\p{Cc}]+/gu, "-").trim();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const safeBaseName = sourceName.replace(/[\x00-\x1f<>:"\/\\|?*]+/g, "-").trim(); | |
| const safeBaseName = sourceName.replace(/[<>:"/\\|?*\p{Cc}]+/gu, "-").trim(); |
🧰 Tools
🪛 Biome (2.4.15)
[error] 366-366: Unexpected control character in a regular expression.
(lint/suspicious/noControlCharactersInRegex)
[error] 366-366: Unexpected control character in a regular expression.
(lint/suspicious/noControlCharactersInRegex)
[error] 366-366: The character doesn't need to be escaped.
(lint/complexity/noUselessEscapeInRegex)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/video-editor/VideoEditor.tsx` at line 366, The regex used to
compute safeBaseName (sourceName.replace(/[\x00-\x1f<>:"\/\\|?*]+/g, "-"))
triggers Biome lint rules for control-character and useless-escape usage;
replace it with a Unicode-aware character-class that uses the control category
and removes unnecessary escapes, e.g. use
sourceName.replace(/[\p{Cc}<>:"\\|?*]+/gu, "-") in the VideoEditor.tsx code
where safeBaseName is computed so it matches all control characters via \p{Cc},
adds the 'u' flag, and no longer contains useless escapes.
Addresses CodeRabbit pre-merge docstring coverage warning.
Author
|
Addressed the docstring coverage warning in 69fb2a8 - added JSDoc to the exported converters ( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.srtor.vttfile, distinct from the burn-in caption render path.electron/ipc/captions/exportSubtitleFile.ts: purecuesToSrt(cues)/cuesToVtt(cues)serializers + an IPC handler that opens the Electron save-dialog and writes the file viafs.promises.writeFile. Picking up where PR feat(captions): export subtitle files #355 left off after @meiiie stepped back to reduce PR noise.electron/ipc/captions/exportSubtitleFile.test.tscovers SRT/VTT formatting, malformed cues (end<start) being clamped, empty cue arrays, multi-line cue text, and format dispatch.VideoEditorexport menu, enabled only whenautoCaptionsis non-empty.cc @meiiie - picking up your 2026-05-09 re-up invitation. The burn-in caption path (
captionRenderer.ts) is untouched.Closes #334.
AI was used for assistance.
Summary by CodeRabbit
New Features
Tests