sachac / subed

subed is a subtitle editor for Emacs
177 stars 16 forks source link

[bug] Calling subed-merge-dwim (M-m) on a subtitle which has a line with only digits makes the digits disappear #71

Open rodrigomorales1 opened 5 days ago

rodrigomorales1 commented 5 days ago

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 to M-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.

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

sachac commented 3 days ago

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 https://github.com/sachac/subed/issues/71, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACD7ETDEZE2EL5CWSHJEALZKIQ4ZAVCNFSM6AAAAABKGWOXXGVHI2DSMVQWIX3LMV43ASLTON2WKOZSGM4DKMJVGMZDCNI . You are receiving this because you are subscribed to this thread.Message ID: @.***>

sachac commented 1 day ago

@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 (https://github.com/sachac/subed/commit/110e4bdfe1fbcb94972adc246d52b071ca95a2f2)?