From 57705c12d25f61a6abf75f9cc3185bdfca61020e Mon Sep 17 00:00:00 2001 From: "Martijn van Berkel (Flitskikker)" Date: Wed, 7 Sep 2022 17:22:05 +0200 Subject: [PATCH] Don't add dashes if there are already dashes, use safe substring, add some unit tests --- src/Test/Logic/ConvertColorsToDialogTest.cs | 322 ++++++++++++++++++ src/Test/Test.csproj | 1 + .../Common/ConvertColorsToDialogUtils.cs | 109 +++--- 3 files changed, 392 insertions(+), 40 deletions(-) create mode 100644 src/Test/Logic/ConvertColorsToDialogTest.cs diff --git a/src/Test/Logic/ConvertColorsToDialogTest.cs b/src/Test/Logic/ConvertColorsToDialogTest.cs new file mode 100644 index 000000000..d0b707dd7 --- /dev/null +++ b/src/Test/Logic/ConvertColorsToDialogTest.cs @@ -0,0 +1,322 @@ +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Nikse.SubtitleEdit.Core.Common; +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Test.Logic +{ + [TestClass] + public class ConvertColorsToDialogTest + { + [TestMethod] + public void TestDialog1() + { + Configuration.Settings.General.DialogStyle = Nikse.SubtitleEdit.Core.Enums.DialogType.DashBothLinesWithSpace; + + var subtitle = new Subtitle(new List() { new Paragraph("That was really delicious." + Environment.NewLine + + "I know.", 0, 2000) }); + + ConvertColorsToDialogUtils.ConvertColorsToDialogInSubtitle(subtitle, true, false, false); + var result = subtitle.Paragraphs.First().Text; + + Assert.AreEqual("- That was really delicious." + Environment.NewLine + "- I know.", result); + } + + [TestMethod] + public void TestDialog2() + { + Configuration.Settings.General.DialogStyle = Nikse.SubtitleEdit.Core.Enums.DialogType.DashBothLinesWithSpace; + + var subtitle = new Subtitle(new List() { new Paragraph("That's it!" + Environment.NewLine + + "..sped to victory.", 0, 2000) }); + + ConvertColorsToDialogUtils.ConvertColorsToDialogInSubtitle(subtitle, true, false, false); + var result = subtitle.Paragraphs.First().Text; + + Assert.AreEqual("- That's it!" + Environment.NewLine + "- ..sped to victory.", result); + } + + [TestMethod] + public void TestDialog2Alt() + { + Configuration.Settings.General.DialogStyle = Nikse.SubtitleEdit.Core.Enums.DialogType.DashSecondLineWithoutSpace; + + var subtitle = new Subtitle(new List() { new Paragraph("That's it!" + Environment.NewLine + + "..sped to victory.", 0, 2000) }); + + ConvertColorsToDialogUtils.ConvertColorsToDialogInSubtitle(subtitle, true, false, false); + var result = subtitle.Paragraphs.First().Text; + + Assert.AreEqual("That's it!" + Environment.NewLine + "-..sped to victory.", result); + } + + [TestMethod] + public void TestDialog3() + { + Configuration.Settings.General.DialogStyle = Nikse.SubtitleEdit.Core.Enums.DialogType.DashBothLinesWithSpace; + + var subtitle = new Subtitle(new List() { new Paragraph("That's it!" + Environment.NewLine + + "..sped to victory.", 0, 2000) }); + + ConvertColorsToDialogUtils.ConvertColorsToDialogInSubtitle(subtitle, true, false, false); + var result = subtitle.Paragraphs.First().Text; + + Assert.AreEqual("- That's it!" + Environment.NewLine + "- ..sped to victory.", result); + } + + [TestMethod] + public void TestDialog3Alt() + { + Configuration.Settings.General.DialogStyle = Nikse.SubtitleEdit.Core.Enums.DialogType.DashSecondLineWithoutSpace; + + var subtitle = new Subtitle(new List() { new Paragraph("That's it!" + Environment.NewLine + + "..sped to victory.", 0, 2000) }); + + ConvertColorsToDialogUtils.ConvertColorsToDialogInSubtitle(subtitle, true, false, false); + var result = subtitle.Paragraphs.First().Text; + + Assert.AreEqual("That's it!" + Environment.NewLine + "-..sped to victory.", result); + } + + [TestMethod] + public void TestDialog4() + { + Configuration.Settings.General.DialogStyle = Nikse.SubtitleEdit.Core.Enums.DialogType.DashBothLinesWithSpace; + + var subtitle = new Subtitle(new List() { new Paragraph("We are going to Ibiza." + Environment.NewLine + + "To where? Ibiza. Okay.", 0, 2000) }); + + ConvertColorsToDialogUtils.ConvertColorsToDialogInSubtitle(subtitle, true, false, false); + var result = subtitle.Paragraphs.First().Text; + + Assert.AreEqual("- We are going to Ibiza." + Environment.NewLine + "- To where? - Ibiza. - Okay.", result); + } + + [TestMethod] + public void TestDialog4Alt() + { + Configuration.Settings.General.DialogStyle = Nikse.SubtitleEdit.Core.Enums.DialogType.DashSecondLineWithoutSpace; + + var subtitle = new Subtitle(new List() { new Paragraph("We are going to Ibiza." + Environment.NewLine + + "To where? Ibiza. Okay.", 0, 2000) }); + + ConvertColorsToDialogUtils.ConvertColorsToDialogInSubtitle(subtitle, true, false, false); + var result = subtitle.Paragraphs.First().Text; + + Assert.AreEqual("We are going to Ibiza." + Environment.NewLine + "-To where? -Ibiza. -Okay.", result); + } + + [TestMethod] + public void TestDialogAddNewLinesOff() + { + Configuration.Settings.General.DialogStyle = Nikse.SubtitleEdit.Core.Enums.DialogType.DashSecondLineWithoutSpace; + + var subtitle = new Subtitle(new List() { new Paragraph("Keep on going. Okay.", 0, 2000) }); + + ConvertColorsToDialogUtils.ConvertColorsToDialogInSubtitle(subtitle, true, false, false); + var result = subtitle.Paragraphs.First().Text; + + Assert.AreEqual("Keep on going. -Okay.", result); + } + + [TestMethod] + public void TestDialogAddNewLinesOn() + { + Configuration.Settings.General.DialogStyle = Nikse.SubtitleEdit.Core.Enums.DialogType.DashSecondLineWithoutSpace; + + var subtitle = new Subtitle(new List() { new Paragraph("Keep on going. Okay.", 0, 2000) }); + + ConvertColorsToDialogUtils.ConvertColorsToDialogInSubtitle(subtitle, true, true, false); + var result = subtitle.Paragraphs.First().Text; + + Assert.AreEqual("Keep on going." + Environment.NewLine + "-Okay.", result); + } + + [TestMethod] + public void TestDialogThreeLines1() + { + Configuration.Settings.General.DialogStyle = Nikse.SubtitleEdit.Core.Enums.DialogType.DashBothLinesWithSpace; + + var subtitle = new Subtitle(new List() { new Paragraph("Got a sinking feeling." + Environment.NewLine + + "\"Evacuate from" + Environment.NewLine + + "an underwater chopper.\" ", 0, 2000) }); + + ConvertColorsToDialogUtils.ConvertColorsToDialogInSubtitle(subtitle, true, false, false); + var result = subtitle.Paragraphs.First().Text; + + Assert.AreEqual("- Got a sinking feeling." + Environment.NewLine + "- \"Evacuate from" + Environment.NewLine + "an underwater chopper.\"", result); + } + + [TestMethod] + public void TestDialogThreeLines2() + { + Configuration.Settings.General.DialogStyle = Nikse.SubtitleEdit.Core.Enums.DialogType.DashBothLinesWithSpace; + + var subtitle = new Subtitle(new List() { new Paragraph("Got a sinking feeling" + Environment.NewLine + + "about all of this." + Environment.NewLine + + "Don't worry. ", 0, 2000) }); + + ConvertColorsToDialogUtils.ConvertColorsToDialogInSubtitle(subtitle, true, false, false); + var result = subtitle.Paragraphs.First().Text; + + Assert.AreEqual("- Got a sinking feeling" + Environment.NewLine + "about all of this." + Environment.NewLine + "- Don't worry.", result); + } + + [TestMethod] + public void TestDialogThreeLines2ReBreak() + { + Configuration.Settings.General.DialogStyle = Nikse.SubtitleEdit.Core.Enums.DialogType.DashBothLinesWithSpace; + + var subtitle = new Subtitle(new List() { new Paragraph("Got a sinking feeling" + Environment.NewLine + + "about all of this." + Environment.NewLine + + "Don't worry. ", 0, 2000) }); + + ConvertColorsToDialogUtils.ConvertColorsToDialogInSubtitle(subtitle, true, false, true); + var result = subtitle.Paragraphs.First().Text; + + Assert.AreEqual("- Got a sinking feeling about all of this." + Environment.NewLine + "- Don't worry.", result); + } + + [TestMethod] + public void TestNoChange1() + { + Configuration.Settings.General.DialogStyle = Nikse.SubtitleEdit.Core.Enums.DialogType.DashBothLinesWithSpace; + + var subtitle = new Subtitle(new List() { new Paragraph("That's it!" + Environment.NewLine + + "..sped to victory.", 0, 2000) }); + + ConvertColorsToDialogUtils.ConvertColorsToDialogInSubtitle(subtitle, true, false, false); + var result = subtitle.Paragraphs.First().Text; + + Assert.AreEqual("That's it!" + Environment.NewLine + "..sped to victory.", result); + } + + [TestMethod] + public void TestNoChange2() + { + Configuration.Settings.General.DialogStyle = Nikse.SubtitleEdit.Core.Enums.DialogType.DashBothLinesWithSpace; + + var subtitle = new Subtitle(new List() { new Paragraph("That's it! Sped to victory.", 0, 2000) }); + + ConvertColorsToDialogUtils.ConvertColorsToDialogInSubtitle(subtitle, true, false, false); + var result = subtitle.Paragraphs.First().Text; + + Assert.AreEqual("That's it! Sped to victory.", result); + } + + [TestMethod] + public void TestNoChange3() + { + Configuration.Settings.General.DialogStyle = Nikse.SubtitleEdit.Core.Enums.DialogType.DashBothLinesWithSpace; + + var subtitle = new Subtitle(new List() { new Paragraph("- That was really delicious." + Environment.NewLine + + "- I know.", 0, 2000) }); + + ConvertColorsToDialogUtils.ConvertColorsToDialogInSubtitle(subtitle, false, false, false); + var result = subtitle.Paragraphs.First().Text; + + Assert.AreEqual("- That was really delicious." + Environment.NewLine + "- I know.", result); + } + + [TestMethod] + public void TestNoChange3NoColor() + { + Configuration.Settings.General.DialogStyle = Nikse.SubtitleEdit.Core.Enums.DialogType.DashBothLinesWithSpace; + + var subtitle = new Subtitle(new List() { new Paragraph("- That was really delicious." + Environment.NewLine + + "- I know.", 0, 2000) }); + + ConvertColorsToDialogUtils.ConvertColorsToDialogInSubtitle(subtitle, true, false, false); + var result = subtitle.Paragraphs.First().Text; + + Assert.AreEqual("- That was really delicious." + Environment.NewLine + "- I know.", result); + } + + [TestMethod] + public void TestNoChange4() + { + Configuration.Settings.General.DialogStyle = Nikse.SubtitleEdit.Core.Enums.DialogType.DashBothLinesWithSpace; + + var subtitle = new Subtitle(new List() { new Paragraph("- That was really delicious." + Environment.NewLine + + "- I know.", 0, 2000) }); + + ConvertColorsToDialogUtils.ConvertColorsToDialogInSubtitle(subtitle, false, false, false); + var result = subtitle.Paragraphs.First().Text; + + Assert.AreEqual("- That was really delicious." + Environment.NewLine + "- I know.", result); + } + + [TestMethod] + public void TestNoChange4NoColor() + { + Configuration.Settings.General.DialogStyle = Nikse.SubtitleEdit.Core.Enums.DialogType.DashBothLinesWithSpace; + + var subtitle = new Subtitle(new List() { new Paragraph("- That was really delicious." + Environment.NewLine + + "- I know.", 0, 2000) }); + + ConvertColorsToDialogUtils.ConvertColorsToDialogInSubtitle(subtitle, true, false, false); + var result = subtitle.Paragraphs.First().Text; + + Assert.AreEqual("- That was really delicious." + Environment.NewLine + "- I know.", result); + } + + [TestMethod] + public void TestNoChange5() + { + Configuration.Settings.General.DialogStyle = Nikse.SubtitleEdit.Core.Enums.DialogType.DashBothLinesWithSpace; + + var subtitle = new Subtitle(new List() { new Paragraph("- No, don't touch that-- - That was stupid." + Environment.NewLine + + "- I know.", 0, 2000) }); + + ConvertColorsToDialogUtils.ConvertColorsToDialogInSubtitle(subtitle, false, false, false); + var result = subtitle.Paragraphs.First().Text; + + Assert.AreEqual("- No, don't touch that-- - That was stupid." + Environment.NewLine + "- I know.", result); + } + + [TestMethod] + public void TestNoChange5NoColor() + { + Configuration.Settings.General.DialogStyle = Nikse.SubtitleEdit.Core.Enums.DialogType.DashBothLinesWithSpace; + + var subtitle = new Subtitle(new List() { new Paragraph("- No, don't touch that-- - That was stupid." + Environment.NewLine + + "- I know.", 0, 2000) }); + + ConvertColorsToDialogUtils.ConvertColorsToDialogInSubtitle(subtitle, true, false, false); + var result = subtitle.Paragraphs.First().Text; + + Assert.AreEqual("- No, don't touch that-- - That was stupid." + Environment.NewLine + "- I know.", result); + } + + [TestMethod] + public void TestDialogInterruption() + { + Configuration.Settings.General.DialogStyle = Nikse.SubtitleEdit.Core.Enums.DialogType.DashBothLinesWithSpace; + + var subtitle = new Subtitle(new List() { new Paragraph("No, don't touch that-- That was stupid." + Environment.NewLine + + "I know.", 0, 2000) }); + + ConvertColorsToDialogUtils.ConvertColorsToDialogInSubtitle(subtitle, false, false, false); + var result = subtitle.Paragraphs.First().Text; + + Assert.AreEqual("- No, don't touch that-- - That was stupid." + Environment.NewLine + "- I know.", result); + } + + [TestMethod] + public void TestDialogInterruptionNoColor() + { + Configuration.Settings.General.DialogStyle = Nikse.SubtitleEdit.Core.Enums.DialogType.DashBothLinesWithSpace; + + var subtitle = new Subtitle(new List() { new Paragraph("No, don't touch that-- That was stupid." + Environment.NewLine + + "I know.", 0, 2000) }); + + ConvertColorsToDialogUtils.ConvertColorsToDialogInSubtitle(subtitle, true, false, false); + var result = subtitle.Paragraphs.First().Text; + + Assert.AreEqual("- No, don't touch that-- - That was stupid." + Environment.NewLine + "- I know.", result); + } + } +} diff --git a/src/Test/Test.csproj b/src/Test/Test.csproj index 6bbbc2220..a6b1216ed 100644 --- a/src/Test/Test.csproj +++ b/src/Test/Test.csproj @@ -65,6 +65,7 @@ + diff --git a/src/libse/Common/ConvertColorsToDialogUtils.cs b/src/libse/Common/ConvertColorsToDialogUtils.cs index 5ed3af8f3..641fd7919 100644 --- a/src/libse/Common/ConvertColorsToDialogUtils.cs +++ b/src/libse/Common/ConvertColorsToDialogUtils.cs @@ -29,10 +29,10 @@ namespace Nikse.SubtitleEdit.Core.Common while (index < p.Text.Length) { - if (index + "", index) - p.Text.IndexOf("=", index) - 1).Replace("\"", ""); + newColor = p.Text.SafeSubstring(p.Text.IndexOf("=", index) + 1, p.Text.IndexOf(">", index) - p.Text.IndexOf("=", index) - 1).Replace("\"", ""); if (currentColor == null) { @@ -40,28 +40,33 @@ namespace Nikse.SubtitleEdit.Core.Common } else if (currentColor != newColor) { - if (dashFirstLine && !firstLineAdded) + // Don't insert dash if there is already a dash, but DO insert a dash if it is an interruption + if (p.Text.SafeSubstring(index, 1) != "-" && p.Text.SafeSubstring(index - 1, 1) != "-" + && (p.Text.SafeSubstring(index - 2, 2) != "- " || p.Text.SafeSubstring(index - 3, 3) == "-- ")) { - p.Text = dash + p.Text; + if (dashFirstLine && !firstLineAdded) + { + p.Text = dash + p.Text; + index += dash.Length; + + firstLineAdded = true; + } + + if (!addNewLines && p.Text.SafeSubstring(index - 1, 1) != " " && p.Text.SafeSubstring(index - 1, 1) != "\r" && p.Text.SafeSubstring(index - 1, 1) != "\n") + { + p.Text = p.Text.SafeSubstring(0, index) + " " + p.Text.SafeSubstring(index); + index += 1; + } + else if (addNewLines && p.Text.SafeSubstring(index - 1, 1) != "\r" && p.Text.SafeSubstring(index - 1, 1) != "\n") + { + p.Text = p.Text.SafeSubstring(0, index) + Environment.NewLine + p.Text.SafeSubstring(index); + index += Environment.NewLine.Length; + } + + p.Text = p.Text.SafeSubstring(0, index) + dash + p.Text.SafeSubstring(index); index += dash.Length; - - firstLineAdded = true; } - if (!addNewLines && p.Text.Substring(index - 1, 1) != " " && p.Text.Substring(index - 1, 1) != "\r" && p.Text.Substring(index - 1, 1) != "\n") - { - p.Text = p.Text.Substring(0, index) + " " + p.Text.Substring(index); - index += 1; - } - else if (addNewLines && p.Text.Substring(index - 1, 1) != "\r" && p.Text.Substring(index - 1, 1) != "\n") - { - p.Text = p.Text.Substring(0, index) + Environment.NewLine + p.Text.Substring(index); - index += Environment.NewLine.Length; - } - - p.Text = p.Text.Substring(0, index) + dash + p.Text.Substring(index); - index += dash.Length; - currentColor = newColor; } @@ -69,14 +74,14 @@ namespace Nikse.SubtitleEdit.Core.Common endOfColor = false; } - else if (index + "".Length <= p.Text.Length && p.Text.Substring(index, "".Length).ToLowerInvariant() == "") + else if (index + "".Length <= p.Text.Length && p.Text.SafeSubstring(index, "".Length).ToLowerInvariant() == "") { // End of color endOfColor = true; index += "".Length; } - else if (index + 1 <= p.Text.Length && p.Text.Substring(index, 1) == " " || p.Text.Substring(index, 1) == "\r" || p.Text.Substring(index, 1) == "\n") + else if (index + 1 <= p.Text.Length && p.Text.SafeSubstring(index, 1) == " " || p.Text.SafeSubstring(index, 1) == "\r" || p.Text.SafeSubstring(index, 1) == "\n") { // Whitespace, ignore index += 1; @@ -96,28 +101,33 @@ namespace Nikse.SubtitleEdit.Core.Common if (currentColor != newColor) { - if (dashFirstLine && !firstLineAdded) + // Don't insert dash if there is already a dash, but DO insert a dash if it is an interruption + if (p.Text.SafeSubstring(index, 1) != "-" && p.Text.SafeSubstring(index - 1, 1) != "-" + && (p.Text.SafeSubstring(index - 2, 2) != "- " || p.Text.SafeSubstring(index - 3, 3) == "-- ")) { - p.Text = dash + p.Text; + if (dashFirstLine && !firstLineAdded) + { + p.Text = dash + p.Text; + index += dash.Length; + + firstLineAdded = true; + } + + if (!addNewLines && p.Text.SafeSubstring(index - 1, 1) != " " && p.Text.SafeSubstring(index - 1, 1) != "\r" && p.Text.SafeSubstring(index - 1, 1) != "\n") + { + p.Text = p.Text.SafeSubstring(0, index) + " " + p.Text.SafeSubstring(index); + index += 1; + } + else if (addNewLines && p.Text.SafeSubstring(index - 1, 1) != "\r" && p.Text.SafeSubstring(index - 1, 1) != "\n") + { + p.Text = p.Text.SafeSubstring(0, index) + Environment.NewLine + p.Text.SafeSubstring(index); + index += Environment.NewLine.Length; + } + + p.Text = p.Text.SafeSubstring(0, index) + dash + p.Text.SafeSubstring(index); index += dash.Length; - - firstLineAdded = true; } - if (!addNewLines && p.Text.Substring(index - 1, 1) != " " && p.Text.Substring(index - 1, 1) != "\r" && p.Text.Substring(index - 1, 1) != "\n") - { - p.Text = p.Text.Substring(0, index) + " " + p.Text.Substring(index); - index += 1; - } - else if (addNewLines && p.Text.Substring(index - 1, 1) != "\r" && p.Text.Substring(index - 1, 1) != "\n") - { - p.Text = p.Text.Substring(0, index) + Environment.NewLine + p.Text.Substring(index); - index += Environment.NewLine.Length; - } - - p.Text = p.Text.Substring(0, index) + dash + p.Text.Substring(index); - index += dash.Length; - currentColor = newColor; } } @@ -147,6 +157,25 @@ namespace Nikse.SubtitleEdit.Core.Common } } + private static string SafeSubstring(this string value, int startIndex, int length = -1, string defaultValue = "") + { + try + { + if (length >= 0) + { + return value.Substring(startIndex, length); + } + else + { + return value.Substring(startIndex); + } + } + catch (ArgumentOutOfRangeException) + { + return defaultValue; + } + } + public static void ConvertColorsToDialogInSubtitle(Subtitle subtitle, bool removeColorTags, bool addNewLines, bool reBreakLines) { var language = LanguageAutoDetect.AutoDetectGoogleLanguage(subtitle);