diff --git a/src/ui/Assets/Languages/English.json b/src/ui/Assets/Languages/English.json index 70ba7979367..653407110d0 100644 --- a/src/ui/Assets/Languages/English.json +++ b/src/ui/Assets/Languages/English.json @@ -1751,7 +1751,11 @@ "unbreakShortLine": "Unbreak short line", "fixText": "Fix text", "removeSpaceBetweenNumbers": "Remove space between numbers", - "fixDialogsOnOneLine": "Fix dialogs on one line" + "fixDialogsOnOneLine": "Fix dialogs on one line", + "fixTypeFormatting": "Formatting", + "fixTypeDialog": "Dialog", + "fixTypePunctuation": "Punctuation", + "fixTypeOcr": "OCR" }, "adjustDurations": { "title": "Adjust durations", diff --git a/src/ui/Features/Tools/FixCommonErrors/FixCommonErrorsViewModel.cs b/src/ui/Features/Tools/FixCommonErrors/FixCommonErrorsViewModel.cs index f9765eb404f..6ecdd26c583 100644 --- a/src/ui/Features/Tools/FixCommonErrors/FixCommonErrorsViewModel.cs +++ b/src/ui/Features/Tools/FixCommonErrors/FixCommonErrorsViewModel.cs @@ -36,6 +36,8 @@ public partial class FixCommonErrorsViewModel : ObservableObject, IFixCallbacks private const int AnalysingPaintDelayMilliseconds = 20; [ObservableProperty] private string _searchText; + [ObservableProperty] private ObservableCollection _fixTypes; + [ObservableProperty] private FixTypeDisplayItem? _selectedFixType; [ObservableProperty] private ObservableCollection _languages; [ObservableProperty] private LanguageDisplayItem? _selectedLanguage; [ObservableProperty] private ObservableCollection _fixes; @@ -116,6 +118,13 @@ public FixCommonErrorsViewModel(INamesList namesList, IWindowService windowServi GridSubtitles = new TableView(); SearchText = string.Empty; + FixTypes = new ObservableCollection { new() }; + foreach (var fixType in Enum.GetValues()) + { + FixTypes.Add(new FixTypeDisplayItem(fixType)); + } + + SelectedFixType = FixTypes[0]; Languages = new ObservableCollection(); Language = new string(' ', 0); Fixes = new ObservableCollection(); @@ -961,7 +970,28 @@ internal void OnKeyDown(KeyEventArgs e) } } - internal void TextBoxSearch_TextChanged(object? sender, TextChangedEventArgs e) + partial void OnSearchTextChanged(string value) + { + RebuildVisibleRules(); + } + + partial void OnSelectedFixTypeChanged(FixTypeDisplayItem? value) + { + RebuildVisibleRules(); + } + + // Runs before PropertyChanged is raised, so the grid rebinds to an already filtered + // collection - a search text or type filter carries over to the newly picked profile. + partial void OnSelectedProfileChanged(ProfileDisplayItem? value) + { + RebuildVisibleRules(); + } + + /// + /// Narrows the step 1 rules grid to the rules matching both the search text and the + /// selected fix type. + /// + internal void RebuildVisibleRules() { if (SelectedProfile == null) { @@ -975,10 +1005,13 @@ internal void TextBoxSearch_TextChanged(object? sender, TextChangedEventArgs e) SelectedProfile.AllFixRules = SelectedProfile.FixRules.ToList(); } + var fixType = SelectedFixType?.FixType; SelectedProfile.FixRules.Clear(); foreach (var rule in SelectedProfile.AllFixRules) { - if (string.IsNullOrEmpty(SearchText) || rule.Name.ToLowerInvariant().Contains(SearchText.ToLowerInvariant())) + var typeMatches = fixType == null || rule.FixType == fixType; + var searchMatches = string.IsNullOrEmpty(SearchText) || rule.Name.Contains(SearchText, StringComparison.OrdinalIgnoreCase); + if (typeMatches && searchMatches) { SelectedProfile.FixRules.Add(rule); } diff --git a/src/ui/Features/Tools/FixCommonErrors/FixCommonErrorsWindow.cs b/src/ui/Features/Tools/FixCommonErrors/FixCommonErrorsWindow.cs index 467de862435..655a5c2a0ba 100644 --- a/src/ui/Features/Tools/FixCommonErrors/FixCommonErrorsWindow.cs +++ b/src/ui/Features/Tools/FixCommonErrors/FixCommonErrorsWindow.cs @@ -51,11 +51,20 @@ public FixCommonErrorsWindow(FixCommonErrorsViewModel vm) labelStep2.Bind(Label.ContentProperty, new Binding(nameof(vm.Step2Title))); labelStep2.Bind(IsVisibleProperty, new Binding(nameof(vm.Step2IsVisible))); - var textBoxSearch = UiUtil.MakeTextBox(250, vm, nameof(vm.SearchText)).WithMarginRight(25) + var textBoxSearch = UiUtil.MakeTextBox(200, vm, nameof(vm.SearchText)).WithMarginRight(25) .WithAccessibleName(Se.Language.Tools.FixCommonErrors.SearchRulesDotDotDot); textBoxSearch.PlaceholderText = Se.Language.Tools.FixCommonErrors.SearchRulesDotDotDot; textBoxSearch.Bind(IsVisibleProperty, new Binding(nameof(vm.Step1IsVisible))); - textBoxSearch.TextChanged += vm.TextBoxSearch_TextChanged; + + // Narrows the rules grid to one FixType; combined with the search text in the view model. + var labelFixType = UiUtil.MakeTextBlock(Se.Language.General.Type).WithMarginRight(5); + labelFixType.Bind(IsVisibleProperty, new Binding(nameof(vm.Step1IsVisible))); + var comboFixType = UiUtil.MakeComboBox(vm.FixTypes, vm, nameof(vm.SelectedFixType)) + .WithMinWidth(120) + .WithMarginRight(25); + comboFixType.Bind(IsVisibleProperty, new Binding(nameof(vm.Step1IsVisible))); + AutomationProperties.SetLabeledBy(comboFixType, labelFixType); + // Off by default (#12441) - keep it reachable here, next to the language it depends on, // instead of only in the OCR window where a Fix-common-errors user would never look. var checkBoxGuessUnknownWords = UiUtil.MakeCheckBox(Se.Language.Ocr.TryToGuessUnknownWords, vm, nameof(vm.TryToGuessUnknownWords)) @@ -69,6 +78,9 @@ public FixCommonErrorsWindow(FixCommonErrorsViewModel vm) HorizontalAlignment = HorizontalAlignment.Right, Children = { + textBoxSearch, + labelFixType, + comboFixType, checkBoxGuessUnknownWords, UiUtil.MakeTextBlock(Se.Language.General.Language).WithMarginRight(5), UiUtil.MakeComboBox(vm.Languages, vm, nameof(vm.SelectedLanguage)) diff --git a/src/ui/Features/Tools/FixCommonErrors/FixRuleDisplayItem.cs b/src/ui/Features/Tools/FixCommonErrors/FixRuleDisplayItem.cs index 5a8b2428dcf..f245c9d92e9 100644 --- a/src/ui/Features/Tools/FixCommonErrors/FixRuleDisplayItem.cs +++ b/src/ui/Features/Tools/FixCommonErrors/FixRuleDisplayItem.cs @@ -1,5 +1,6 @@ using CommunityToolkit.Mvvm.ComponentModel; using Nikse.SubtitleEdit.Core.Common; +using Nikse.SubtitleEdit.Core.Enums; using Nikse.SubtitleEdit.Core.Forms.FixCommonErrors; using Nikse.SubtitleEdit.Core.Interfaces; using Nikse.SubtitleEdit.Logic.Config; @@ -19,6 +20,23 @@ public partial class FixRuleDisplayItem : ObservableObject public string FixCommonErrorFunctionName { get; set; } + /// + /// The kind of fix the rule performs, taken from the fix class itself so the category + /// has one source of truth. Null when does not + /// name a known fix - such a rule is only listed under "All" in the type filter. + /// + public FixType? FixType { get; private set; } + + // Built once: GetFixCommonErrorItems() news up every fix class, and the copy-ctor runs + // per rule per profile. + private static readonly Lazy> FixTypesByFunctionName = new(() => + GetFixCommonErrorItems().ToDictionary(p => p.GetType().Name, p => p.FixType, StringComparer.Ordinal)); + + public static bool TryResolveFixType(string fixCommonErrorFunctionName, out FixType fixType) + { + return FixTypesByFunctionName.Value.TryGetValue(fixCommonErrorFunctionName, out fixType); + } + public FixRuleDisplayItem() { Name = string.Empty; @@ -33,6 +51,7 @@ public FixRuleDisplayItem(FixRuleDisplayItem item) IsSelected = item.IsSelected; SortOrder = item.SortOrder; FixCommonErrorFunctionName = item.FixCommonErrorFunctionName; + FixType = item.FixType; } public FixRuleDisplayItem(string name, string example, int sortOrder, bool isSelected, string fixCommonErrorFunctionName) @@ -42,6 +61,7 @@ public FixRuleDisplayItem(string name, string example, int sortOrder, bool isSel SortOrder = sortOrder; IsSelected = isSelected; FixCommonErrorFunctionName = fixCommonErrorFunctionName; + FixType = TryResolveFixType(fixCommonErrorFunctionName, out var fixType) ? fixType : null; } public IFixCommonError GetFixCommonErrorFunction() diff --git a/src/ui/Features/Tools/FixCommonErrors/FixTypeDisplayItem.cs b/src/ui/Features/Tools/FixCommonErrors/FixTypeDisplayItem.cs new file mode 100644 index 00000000000..f658cd5772a --- /dev/null +++ b/src/ui/Features/Tools/FixCommonErrors/FixTypeDisplayItem.cs @@ -0,0 +1,31 @@ +using Nikse.SubtitleEdit.Core.Enums; +using Nikse.SubtitleEdit.Logic.Config; + +namespace Nikse.SubtitleEdit.Features.Tools.FixCommonErrors; + +/// +/// One entry in the step 1 "Type" filter combo. FixType == null means "all types". +/// +public class FixTypeDisplayItem +{ + public FixType? FixType { get; } + public string Name { get; } + + public FixTypeDisplayItem(FixType fixType) + { + FixType = fixType; + Name = Se.Language.Tools.FixCommonErrors.GetFixTypeName(fixType); + } + + public FixTypeDisplayItem() // "All" entry + { + FixType = null; + Name = Se.Language.General.All; + } + + // The combo uses the default item template, which shows ToString(). + public override string ToString() + { + return Name; + } +} diff --git a/src/ui/Logic/Config/Language/Tools/LanguageFixCommonErrors.cs b/src/ui/Logic/Config/Language/Tools/LanguageFixCommonErrors.cs index dfa4974b898..5f568e6af37 100644 --- a/src/ui/Logic/Config/Language/Tools/LanguageFixCommonErrors.cs +++ b/src/ui/Logic/Config/Language/Tools/LanguageFixCommonErrors.cs @@ -1,4 +1,6 @@ -namespace Nikse.SubtitleEdit.Logic.Config.Language.Tools; +using Nikse.SubtitleEdit.Core.Enums; + +namespace Nikse.SubtitleEdit.Logic.Config.Language.Tools; public class LanguageFixCommonErrors { @@ -125,6 +127,10 @@ public class LanguageFixCommonErrors public string FixText { get; set; } public string RemoveSpaceBetweenNumbers { get; set; } public string FixDialogsOnOneLine { get; set; } + public string FixTypeFormatting { get; set; } + public string FixTypeDialog { get; set; } + public string FixTypePunctuation { get; set; } + public string FixTypeOcr { get; set; } public LanguageFixCommonErrors() { @@ -253,5 +259,25 @@ public LanguageFixCommonErrors() FixText = "Fix text"; RemoveSpaceBetweenNumbers = "Remove space between numbers"; FixDialogsOnOneLine = "Fix dialogs on one line"; + FixTypeFormatting = "Formatting"; + FixTypeDialog = "Dialog"; + FixTypePunctuation = "Punctuation"; + FixTypeOcr = "OCR"; + } + + public string GetFixTypeName(FixType fixType) + { + return fixType switch + { + Core.Enums.FixType.Time => Se.Language.General.Time, + Core.Enums.FixType.Formatting => FixTypeFormatting, + Core.Enums.FixType.Dialog => FixTypeDialog, + Core.Enums.FixType.Punctuation => FixTypePunctuation, + Core.Enums.FixType.Casing => Se.Language.General.Casing, + Core.Enums.FixType.Spacing => Se.Language.General.Spacing, + Core.Enums.FixType.Characters => Se.Language.General.Characters, + Core.Enums.FixType.Ocr => FixTypeOcr, + _ => fixType.ToString(), + }; } } \ No newline at end of file diff --git a/tests/UI/Features/Tools/FixCommonErrors/FixCommonErrorsRuleFilterTests.cs b/tests/UI/Features/Tools/FixCommonErrors/FixCommonErrorsRuleFilterTests.cs new file mode 100644 index 00000000000..05d98875231 --- /dev/null +++ b/tests/UI/Features/Tools/FixCommonErrors/FixCommonErrorsRuleFilterTests.cs @@ -0,0 +1,212 @@ +using Avalonia.Controls; +using Avalonia.Headless.XUnit; +using Avalonia.LogicalTree; +using Nikse.SubtitleEdit.Core.Enums; +using Nikse.SubtitleEdit.Core.Forms.FixCommonErrors; +using Nikse.SubtitleEdit.Features.Tools.FixCommonErrors; +using Nikse.SubtitleEdit.Logic; +using Nikse.SubtitleEdit.Logic.Config; + +namespace UITests.Features.Tools.FixCommonErrors; + +// Step 1 lists ~38 rules flat. The "Type" combo narrows the grid to one FixType, ANDed with +// the search text. Both filter from the profile's full list (AllFixRules) and never from the +// grid collection, so hidden rules keep their selection and reappear intact. +public class FixCommonErrorsRuleFilterTests : IDisposable +{ + private readonly List _windows = new(); + + public void Dispose() + { + foreach (var window in _windows) + { + window.Close(); + } + + _windows.Clear(); + } + + // WindowService only touches the provider when it creates a child window, which the + // construction test never does. + private sealed class NullServiceProvider : IServiceProvider + { + public object? GetService(Type serviceType) => null; + } + + private static List MakeMixedRules() + { + return new List + { + new("Fix commas", string.Empty, 1, true, nameof(FixCommas)), // Punctuation + new("Fix short gaps", string.Empty, 1, true, nameof(FixShortGaps)), // Time + new("Remove empty lines", string.Empty, 1, true, nameof(FixEmptyLines)), // Formatting + }; + } + + private static ProfileDisplayItem MakeProfile(string name) + { + var rules = MakeMixedRules(); + return new ProfileDisplayItem + { + Name = name, + FixRules = new System.Collections.ObjectModel.ObservableCollection(rules), + AllFixRules = rules, + }; + } + + private static FixCommonErrorsViewModel BuildViewModel(out ProfileDisplayItem profile) + { + var vm = new FixCommonErrorsViewModel(null!, null!, null!); + profile = MakeProfile("Default"); + vm.Profiles.Add(profile); + vm.SelectedProfile = profile; + return vm; + } + + private static FixTypeDisplayItem TypeItem(FixCommonErrorsViewModel vm, FixType fixType) + { + return vm.FixTypes.First(p => p.FixType == fixType); + } + + [AvaloniaFact] + public void FixTypes_StartsWithAll_ThenEveryEnumMember() + { + var vm = new FixCommonErrorsViewModel(null!, null!, null!); + + Assert.Null(vm.FixTypes[0].FixType); + Assert.Equal(Enum.GetValues().Length + 1, vm.FixTypes.Count); + Assert.Same(vm.FixTypes[0], vm.SelectedFixType); + } + + [AvaloniaFact] + public void SelectingType_ShowsOnlyRulesOfThatType() + { + var vm = BuildViewModel(out var profile); + + vm.SelectedFixType = TypeItem(vm, FixType.Time); + + var rule = Assert.Single(profile.FixRules); + Assert.Equal(nameof(FixShortGaps), rule.FixCommonErrorFunctionName); + Assert.Equal(3, profile.AllFixRules.Count); + } + + [AvaloniaFact] + public void SelectingAll_RestoresEveryRule() + { + var vm = BuildViewModel(out var profile); + vm.SelectedFixType = TypeItem(vm, FixType.Time); + + vm.SelectedFixType = vm.FixTypes[0]; + + Assert.Equal(profile.AllFixRules, profile.FixRules); + } + + [AvaloniaFact] + public void TypeAndSearch_AreCombined() + { + var vm = BuildViewModel(out var profile); + + vm.SelectedFixType = TypeItem(vm, FixType.Punctuation); + vm.SearchText = "gap"; + Assert.Empty(profile.FixRules); + + vm.SelectedFixType = TypeItem(vm, FixType.Time); + Assert.Single(profile.FixRules); + + vm.SearchText = string.Empty; + Assert.Single(profile.FixRules); // the type filter alone still applies + } + + [AvaloniaFact] + public void SwitchingProfile_KeepsActiveFilter() + { + var vm = BuildViewModel(out _); + var other = MakeProfile("Other"); + vm.Profiles.Add(other); + vm.SelectedFixType = TypeItem(vm, FixType.Time); + + vm.SelectedProfile = other; + + var rule = Assert.Single(other.FixRules); + Assert.Equal(nameof(FixShortGaps), rule.FixCommonErrorFunctionName); + } + + [AvaloniaFact] + public void HiddenRules_KeepTheirSelection() + { + var vm = BuildViewModel(out var profile); + var commas = profile.AllFixRules.First(p => p.FixCommonErrorFunctionName == nameof(FixCommas)); + vm.SelectedFixType = TypeItem(vm, FixType.Time); + + vm.RulesInverseSelected(); // acts on the visible (filtered) rows only + vm.SelectedFixType = vm.FixTypes[0]; + + Assert.True(commas.IsSelected); + Assert.False(profile.AllFixRules.First(p => p.FixCommonErrorFunctionName == nameof(FixShortGaps)).IsSelected); + } + + [Fact] + public void MakeDefaultRules_EveryRuleResolvesItsFixType() + { + var rules = FixCommonErrorsViewModel.MakeDefaultRules(); + + Assert.NotEmpty(rules); + Assert.All(rules, rule => Assert.NotNull(rule.FixType)); + } + + [Theory] + [InlineData(nameof(FixEllipsesStart), FixType.Punctuation)] + [InlineData(nameof(FixAloneLowercaseIToUppercaseI), FixType.Casing)] + [InlineData(nameof(FixTurkishAnsiToUnicode), FixType.Characters)] + [InlineData(nameof(FixDanishLetterI), FixType.Casing)] + [InlineData(nameof(FixSpanishInvertedQuestionAndExclamationMarks), FixType.Punctuation)] + [InlineData(nameof(FixCommonOcrErrors), FixType.Ocr)] + public void LanguageSpecificRules_ResolveTheirFixType(string functionName, FixType expected) + { + Assert.True(FixRuleDisplayItem.TryResolveFixType(functionName, out var fixType)); + Assert.Equal(expected, fixType); + } + + [Fact] + public void UnknownRuleName_HasNoFixType() + { + var rule = new FixRuleDisplayItem("Unknown", string.Empty, 1, true, "NoSuchFix"); + + Assert.Null(rule.FixType); + } + + [Fact] + public void CopyConstructor_CopiesFixType() + { + var original = new FixRuleDisplayItem("Fix commas", string.Empty, 1, true, nameof(FixCommas)); + + var copy = new FixRuleDisplayItem(original); + + Assert.Equal(FixType.Punctuation, copy.FixType); + } + + // The search box used to be created but never added to the window, so it was invisible. + // Both it and the type combo live in the step 1 toolbar and hide with it. + [AvaloniaFact] + public void Window_ShowsSearchBoxAndTypeCombo_OnlyInStep1() + { + var vm = new FixCommonErrorsViewModel(null!, new WindowService(new NullServiceProvider()), null!); + var window = new FixCommonErrorsWindow(vm); + _windows.Add(window); + + var search = window.GetLogicalDescendants().OfType() + .FirstOrDefault(p => p.PlaceholderText == Se.Language.Tools.FixCommonErrors.SearchRulesDotDotDot); + var combo = window.GetLogicalDescendants().OfType() + .FirstOrDefault(p => ReferenceEquals(p.ItemsSource, vm.FixTypes)); + Assert.NotNull(search); + Assert.NotNull(combo); + Assert.True(search.IsVisible); + Assert.True(combo.IsVisible); + Assert.Same(vm.FixTypes[0], combo.SelectedItem); + + vm.Step1IsVisible = false; + + Assert.False(search.IsVisible); + Assert.False(combo.IsVisible); + } +}