fix: deduplicate secondary subtitle lines - #173
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses duplicated text when collecting/combining secondary subtitles (e.g., when one subtitle expands and repeats the previous line), reducing the need for manual cleanup during export.
Changes:
- Add an optional
deduplicate_linesmode tosubtitles.sub_listto remove duplicated secondary lines while concatenating. - Enable line deduplication for secondary subtitle buffers in
subtitles/observer.lua. - Add basic regression tests for subtitle list concatenation behavior in
tests/run.lua.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
mpvacious/subtitles/sub_list.lua |
Adds optional line-deduplication behavior when concatenating subtitle text. |
mpvacious/subtitles/observer.lua |
Enables the new deduplication mode for secondary subtitle collections used during export. |
tests/run.lua |
Adds subtitle list tests covering duplicated-line behavior for secondary subtitles. |
馃挕 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
ae4c9a8 to
bc6748f
Compare
bc6748f to
664c573
Compare
|
|
extract make_speech_collector add function list_equal() add speech collector simplify tests update comments add tests
4b95adb to
c9d9a44
Compare
|
I did everything I could to refactor the code and make it understandable. But commit 4b95adb is left out for now. I'm not sure if we need something like that. Better discuss it separately. |
|
I can extract it in a separate mr where others can weigh in on whether this functionality is needed in the first place |
|
If a PR is clean, I can merge it right away. But when a PR is not clean enough to meet the repo's code quality bar, it takes a lot of work to refactor it before merging. |
There's a problem when text get duplicated between two subtitles. Then I have to remove the duplicated text manually which is a pain. The other thing that annoys me is if I have Japanese cue with, let's say, 3 corresponding english cues, I have to manually select these overlapping english cues. This mr simply grabs all the corresponding english cues that fall into the range of the japanese cue.