[bug] Calling subed-merge-dwim (M-m) on a subtitle which has a line with only digits makes the digits disappear #71
Comments
|
Hmm, I wonder if it's getting misrecognized as a subtitle ID. I'd love to
fix it, although I should manage expectations as parenting doesn't leave me
with a lot of focused time these days. I'll try to squeeze in this and the
other issues at some point! Thanks for your patience!
…On Tue, Jul 2, 2024, 00:02 Rodrigo Morales ***@***.***> wrote:
- Steps to reproduce the bug <#m_-2362693322810534924_orgb844e4b>
- What happened? <#m_-2362693322810534924_org795efa1>
- What should have happened? <#m_-2362693322810534924_orgb187c16>
- Additional information <#m_-2362693322810534924_org72b430a>
Brief description of the bug
When a subtitle has multiple lines, and the last line of that subtitle
only contains digits and subed-merge-dwim is called on that subtitle, the
line with only digits disappear.
Steps to reproduce the bug
I cloned the latest commit of the repository to make sure that I was using
the latest changes.
rm -rf /tmp/subed && \
mkdir /tmp/subed && \
git clone --branch main --depth 1 https://github.com/sachac/subed /tmp/subed/subed-repo
I inserted the content of the code block below to the file
/tmp/subed/a.srt.
1
00:00:00,000 --> 00:00:01,000
This is first subtitle.
2
00:00:01,000 --> 00:00:02,000
This is the second subtitle.
3
00:00:02,000 --> 00:00:03,000
This is the third subtitle.
4
00:00:03,000 --> 00:00:04,000
This is the fourth subtitle.
5
00:00:04,000 --> 00:00:05,000
123456789
6
00:00:05,000 --> 00:00:06,000
This is the sixth subtitle.
I compiled GNU Emacs 29.4 (latest release as of time of writing) and
started Emacs using the command in the code block below.
I set EMACSLOADPATH and EMACSNATIVELOADPATH to an empty value to avoid my
system configuration from modifying the default behavior of Emacs.
cd /my-storage/miscellaneous/source-code-popular/emacs && \
EMACSLOADPATH= EMACSNATIVELOADPATH= ./src/emacs \
--quick \
--no-splash \
--eval "(add-to-list 'load-path \"/tmp/subed/subed-repo/subed/\")" \
--eval "(require 'subed-srt)" \
--eval "(find-file \"/tmp/subed/a.srt\")"
When the Emacs window was opened, the point was at the beginning of the
buffer. I moved the point to the line This is the first subtitle. I
pressed M-m (by default, bound to subed-merge-dwim) twice. The contents
of the buffer a.srt after performing that operation are shown below:
1
00:00:00,000 --> 00:00:03,000
This is first subtitle.
This is the second subtitle.
This is the third subtitle.
2
00:00:03,000 --> 00:00:04,000
This is the fourth subtitle.
3
00:00:04,000 --> 00:00:05,000
123456789
4
00:00:05,000 --> 00:00:06,000
This is the sixth subtitle.
I then moved the point to the line This is the fourth subtitle and called
M-m (by default, bound to subed-merge-dwim) once. The contents of the
buffer a.srt after performing that operation are shown below:
1
00:00:00,000 --> 00:00:03,000
This is first subtitle.
This is the second subtitle.
This is the third subtitle.
2
00:00:03,000 --> 00:00:05,000
This is the fourth subtitle.
123456789
3
00:00:05,000 --> 00:00:05,800
This is the sixth subtitle.
I pressed M-w again and the contents of the buffer were:
1
00:00:00,000 --> 00:00:03,000
This is first subtitle.
This is the second subtitle.
This is the third subtitle.
2
00:00:03,000 --> 00:00:05,800
This is the fourth subtitle.
This is the sixth subtitle.
What happened?
The subtitle whose content was 123456789 got deleted when calling
subed-merge-dwim.
What should have happened?
The subtitle whose content was 12345789 should not have been deleted
after calling subed-merge-dwim. That is, the content of the buffer should
have been the following:
1
00:00:00,000 --> 00:00:03,000
This is first subtitle.
This is the second subtitle.
This is the third subtitle.
2
00:00:03,000 --> 00:00:05,800
This is the fourth subtitle.
1234566789
This is the sixth subtitle.
Additional information
1.
I found this bug because I was editing the subtitles for an audio that
belongs to a chapter in a book on learning Spanish. In that chapter,
students are introduced to how numbers are read in Spanish, so in the audio
the speaker only mentions numbers and the subtitles contain the digits that
correspond to those numbers. I believe it is not unusual for a subtitle to
only contain numbers. For this reason, subed should handle subtitles
that only contain digits properly.
2.
I opened the file and I loaded the theme modus-vivendi. I noticed that
the content of the subtitle with index 5 was shown with the same face
for the index of the subtitles. See screenshot below.
image.png (view on web)
<https://github.com/sachac/subed/assets/74389646/83960b3d-c205-45a7-9027-42c45c09a03d>
—
Reply to this email directly, view it on GitHub
<#71>, or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACD7ETDEZE2EL5CWSHJEALZKIQ4ZAVCNFSM6AAAAABKGWOXXGVHI2DSMVQWIX3LMV43ASLTON2WKOZSGM4DKMJVGMZDCNI>
.
You are receiving this because you are subscribed to this thread.Message
ID: ***@***.***>
|
sachac
added a commit
that referenced
this issue
Jul 5, 2024
*
subed/subed-srt.el (subed--jump-to-subtitle-end):
Handle blank subtitles as a separate case so that
we can use subed-srt--regexp-separator to handle
subtitles with numbers in the cue text.
*
subed/subed-vtt.el (subed--jump-to-subtitle-end):
Update the comment.
* tests/test-subed-srt.el ("SRT"): Add a test case
for jumping to the end of a cue with a line that's
all numbers, and add a test for merging those cues
too.
* tests/test-subed-vtt.el ("VTT"): Add a test case
for the same scenario.
Thanks to rodrigomorales1 for the detailed bug
description!
#71
|
@rodrigomorales1 I think I fixed subed-jump-to-subtitle-end and that should make merging work properly now. =) Would you like to try 1.2.12 (110e4bd)? |
|
I think this works, but please let me know if it doesn't! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Brief description of the bug
When a subtitle has multiple lines, and the last line of that subtitle only contains digits and
subed-merge-dwim(by default, bound toM-m) is called on that subtitle, the line with only digits disappear.Steps to reproduce the bug
I cloned the latest commit of the repository to make sure that I was using the latest changes.
I inserted the content of the code block below to the file
/tmp/subed/a.srt.I compiled GNU Emacs 29.4 (latest release as of time of writing) and started Emacs using the command in the code block below.
I set
EMACSLOADPATHandEMACSNATIVELOADPATHto an empty value to avoid my system configuration from modifying the default behavior of Emacs.When the Emacs window was opened, the point was at the beginning of the buffer. I moved the point to the line
This is the first subtitle. I pressedM-m(by default, bound tosubed-merge-dwim) twice. The contents of the buffera.srtafter performing that operation are shown below:I then moved the point to the line
This is the fourth subtitleand calledM-m(by default, bound tosubed-merge-dwim) once. The contents of the buffera.srtafter performing that operation are shown below:I pressed
M-wagain and the contents of the buffer were:What happened?
The subtitle whose content was
123456789got deleted when callingsubed-merge-dwim.What should have happened?
The subtitle whose content was
12345789should not have been deleted after callingsubed-merge-dwim. That is, the content of the buffer should have been the following:Additional information
I found this bug because I was editing the subtitles for an audio that belongs to a chapter in a book on learning Spanish. In that chapter, students are introduced to how numbers are read in Spanish, so in the audio the speaker only mentions numbers and the subtitles contain the digits that correspond to those numbers. I believe it is not unusual for a subtitle to only contain numbers. For this reason,
subedshould handle subtitles that only contain digits properly.I opened the file and I loaded the theme
modus-vivendi. I noticed that the content of the subtitle with index5was shown with the same face for the index of the subtitles. See screenshot below.The text was updated successfully, but these errors were encountered: