diff --git a/src/LogExpert.Core/Classes/JsonConverters/EncodingJsonConverter.cs b/src/LogExpert.Core/Classes/JsonConverters/EncodingJsonConverter.cs index 6d716de1e..47a6892c8 100644 --- a/src/LogExpert.Core/Classes/JsonConverters/EncodingJsonConverter.cs +++ b/src/LogExpert.Core/Classes/JsonConverters/EncodingJsonConverter.cs @@ -1,6 +1,8 @@ using System; using System.Text; +using LogExpert.Core.Helpers; + using Newtonsoft.Json; namespace LogExpert.Core.Classes.JsonConverters; @@ -53,19 +55,6 @@ public override void WriteJson (JsonWriter writer, object? value, JsonSerializer return null; } - var encodingName = reader.Value?.ToString(); - if (string.IsNullOrEmpty(encodingName)) - { - return Encoding.Default; - } - - try - { - return Encoding.GetEncoding(encodingName); - } - catch (ArgumentException) - { - return Encoding.Default; - } + return EncodingRegistry.GetEncoding(reader.Value?.ToString(), Encoding.Default); } } diff --git a/src/LogExpert.Core/Classes/Log/LineOffsetIndex.cs b/src/LogExpert.Core/Classes/Log/LineOffsetIndex.cs deleted file mode 100644 index 34c9024f6..000000000 --- a/src/LogExpert.Core/Classes/Log/LineOffsetIndex.cs +++ /dev/null @@ -1,53 +0,0 @@ -namespace LogExpert.Core.Classes.Log; - -/// -/// Stores byte offsets for each line start in a file. Supports incremental appending for tail mode. -/// -internal sealed class LineOffsetIndex (int initialCapacity = 4096) -{ - private long[] _offsets = new long[initialCapacity]; - - public int LineCount { get; private set; } - - /// - /// Appends a line-start offset. - /// - public void Add (long offset) - { - if (LineCount == _offsets.Length) - { - Array.Resize(ref _offsets, _offsets.Length * 2); - } - - _offsets[LineCount++] = offset; - } - - /// - /// Returns the byte offset of the start of the given line. - /// - public long GetOffset (int lineNum) - { - return (uint)lineNum < (uint)LineCount ? _offsets[lineNum] : -1; - } - - /// - /// Returns the byte length of the given line (from its start to the next line's start). - /// For the last line, returns -1 (unknown length, read to end or newline). - /// - public long GetLineLength (int lineNum) - { - return (uint)lineNum >= (uint)LineCount - ? -1 - : lineNum + 1 < LineCount - ? _offsets[lineNum + 1] - _offsets[lineNum] - : -1; - } - - /// - /// Removes all offsets, resetting the index. - /// - public void Clear () - { - LineCount = 0; - } -} diff --git a/src/LogExpert.Core/Classes/Log/LogfileReader.cs b/src/LogExpert.Core/Classes/Log/LogfileReader.cs index a0c1fab79..9e2495d9e 100644 --- a/src/LogExpert.Core/Classes/Log/LogfileReader.cs +++ b/src/LogExpert.Core/Classes/Log/LogfileReader.cs @@ -37,8 +37,6 @@ public partial class LogfileReader : ILogfileReader, IMultiFileNavigation, ILogf private readonly ILoadProgressReporter _progressReporter; - private readonly MemoryMappedFileReader _mmfReader; - private const int WAIT_TIME = 1000; private bool _contentDeleted; @@ -186,18 +184,6 @@ private LogfileReader ( _watchedILogFileInfo = fileInfo; - if (!IsMultiFile && _watchedILogFileInfo.Uri?.Scheme is null or "file") - { - try - { - _mmfReader = new MemoryMappedFileReader(_watchedILogFileInfo.FullName, EncodingOptions.Encoding ?? Encoding.Default); - } - catch (IOException) - { - _mmfReader = null; // fallback to buffer path - } - } - StartGCThread(); } @@ -959,12 +945,6 @@ private ValueTask GetLogLineMemoryInternal (int lineNum) return default; } - if (_mmfReader != null && lineNum < _mmfReader.LineCount) - { - var line = _mmfReader.GetLine(lineNum); - return new ValueTask(line); - } - using var readLock = BufferIndex.AcquireReadLock(); { var logBufferEntry = BufferIndex.GetBufferForLineWithIndex(lineNum); @@ -1568,8 +1548,6 @@ private void FileChanged () _logger.Info(CultureInfo.InvariantCulture, "file size changed. new size={0}, file: {1}", newSize, _fileName); FireChangeEvent(); } - - _mmfReader?.ExtendIndex(); } /// @@ -1874,8 +1852,6 @@ protected virtual void Dispose (bool disposing) _cts.Dispose(); BufferIndex.Dispose(); _progressReporter.Dispose(); - _mmfReader?.Dispose(); - } _disposed = true; diff --git a/src/LogExpert.Core/Classes/Log/MemoryMappedFileReader.cs b/src/LogExpert.Core/Classes/Log/MemoryMappedFileReader.cs deleted file mode 100644 index 73f784553..000000000 --- a/src/LogExpert.Core/Classes/Log/MemoryMappedFileReader.cs +++ /dev/null @@ -1,146 +0,0 @@ -using System.Buffers; -using System.IO.MemoryMappedFiles; -using System.Text; - -using ColumnizerLib; - -namespace LogExpert.Core.Classes.Log; - -/// -/// Reads log lines via memory-mapped file access. Builds a line-offset index on load. -/// Supports tail mode by re-mapping when the file grows. -/// -internal sealed class MemoryMappedFileReader (string filePath, Encoding encoding) : IDisposable -{ - private readonly Encoding _encoding = encoding; - private readonly LineOffsetIndex _lineIndex = new(); - private MemoryMappedFile _mmf; - private MemoryMappedViewAccessor _accessor; - private long _mappedLength; - private readonly string _filePath = filePath; - - public int LineCount => _lineIndex.LineCount; - - /// - /// Builds (or rebuilds) the line-offset index by scanning for newline characters. - /// In tail mode, call with startOffset = previously mapped length to index only new content. - /// - public void BuildIndex (long startOffset = 0) - { - using var fs = new FileStream(_filePath, FileMode.Open, FileAccess.Read, FileShare.ReadWrite); - var fileLength = fs.Length; - - if (startOffset == 0) - { - _lineIndex.Clear(); - _lineIndex.Add(0); // first line starts at offset 0 - } - - fs.Position = startOffset; - var buffer = ArrayPool.Shared.Rent(81920); - try - { - int bytesRead; - var position = startOffset; - while ((bytesRead = fs.Read(buffer, 0, buffer.Length)) > 0) - { - for (var i = 0; i < bytesRead; i++) - { - if (buffer[i] == (byte)'\n') - { - _lineIndex.Add(position + i + 1); - } - } - - position += bytesRead; - } - } - finally - { - ArrayPool.Shared.Return(buffer); - } - - // Re-map the file - RemapFile(fileLength); - } - - /// - /// Extends the mapping to cover new file content (for tail mode). - /// - public void ExtendIndex () - { - BuildIndex(_mappedLength); - } - - /// - /// Reads a single line by its zero-based line number. - /// Returns the line as an ILogLineMemory. - /// - public ILogLineMemory GetLine (int lineNum) - { - var offset = _lineIndex.GetOffset(lineNum); - if (offset < 0 || _accessor == null) - { - return null; - } - - var length = _lineIndex.GetLineLength(lineNum); - if (length < 0) - { - // Last line — read to the end of file or a reasonable limit - length = Math.Min(_mappedLength - offset, 1024 * 1024); - } - - if (length <= 0) - { - return new LogLine(ReadOnlyMemory.Empty, lineNum); - } - - // Read bytes from the mapped view - var bytes = new byte[length]; - _ = _accessor.ReadArray(offset, bytes, 0, (int)length); - - // Trim trailing \r\n - var end = (int)length; - if (end > 0 && bytes[end - 1] == '\n') - { - end--; - } - - if (end > 0 && bytes[end - 1] == '\r') - { - end--; - } - - var text = _encoding.GetString(bytes, 0, end); - return new LogLine(text, lineNum); - } - - private void RemapFile (long fileLength) - { - _accessor?.Dispose(); - _mmf?.Dispose(); - - if (fileLength == 0) - { - _mappedLength = 0; - return; - } - - _mmf = MemoryMappedFile.CreateFromFile( - _filePath, - FileMode.Open, - mapName: null, - capacity: fileLength, - MemoryMappedFileAccess.Read); - - _accessor = _mmf.CreateViewAccessor(0, fileLength, MemoryMappedFileAccess.Read); - _mappedLength = fileLength; - } - - public void Dispose () - { - _accessor?.Dispose(); - _mmf?.Dispose(); - } -} diff --git a/src/LogExpert.Core/Classes/Persister/PersisterXML.cs b/src/LogExpert.Core/Classes/Persister/PersisterXML.cs index c570cd5bf..60b122fe5 100644 --- a/src/LogExpert.Core/Classes/Persister/PersisterXML.cs +++ b/src/LogExpert.Core/Classes/Persister/PersisterXML.cs @@ -6,6 +6,7 @@ using LogExpert.Core.Classes.Filter; using LogExpert.Core.Entities; +using LogExpert.Core.Helpers; using NLog; @@ -266,26 +267,24 @@ private static PersistenceData ReadPersistenceDataFromNode (XmlNode node) private static Encoding ReadEncoding (XmlElement fileElement) { XmlNode encodingNode = fileElement.SelectSingleNode("encoding"); - if (encodingNode != null) + if (encodingNode == null) { - XmlAttribute encAttr = encodingNode.Attributes["name"]; - try - { - return encAttr == null ? null : Encoding.GetEncoding(encAttr.Value); - } - catch (ArgumentException e) - { - _logger.Error(e); - return Encoding.Default; - } - catch (NotSupportedException e) - { - _logger.Error(e); - return Encoding.Default; - } + return null; + } + + XmlAttribute encAttr = encodingNode.Attributes["name"]; + if (encAttr == null) + { + return null; + } + + if (EncodingRegistry.TryGetEncoding(encAttr.Value, out var encoding)) + { + return encoding; } - return null; + _logger.Error($"Persisted encoding '{encAttr.Value}' is not supported, falling back to the default encoding"); + return Encoding.Default; } /// diff --git a/src/LogExpert.Core/Helpers/EncodingRegistry.cs b/src/LogExpert.Core/Helpers/EncodingRegistry.cs new file mode 100644 index 000000000..716e4f67c --- /dev/null +++ b/src/LogExpert.Core/Helpers/EncodingRegistry.cs @@ -0,0 +1,115 @@ +using System.Diagnostics.CodeAnalysis; +using System.Text; + +namespace LogExpert.Core.Helpers; + +/// +/// Resolves encoding names and code pages, including the legacy Windows code pages that .NET does not +/// ship with by default. +/// +/// +/// .NET only knows Unicode, ASCII and latin1 out of the box; anything else — windows-1250, +/// windows-1252, … — requires to be registered first, and +/// throws until it is. +/// +/// Registration used to happen as a side effect of constructing the Preferences dialog. Everything +/// that resolves an encoding name runs earlier than that or never opens the dialog at all — the +/// Preferences default encoding, the per-file encoding in a .lxp, the settings JSON — and every one of +/// those call sites swallows the exception and falls back to . The result +/// was that a code page the user had picked was silently discarded on the next start. +/// +/// +/// Resolving through this class removes the ordering problem: every method here registers the provider +/// before it resolves, so no caller has to run after some other component. Callers should not use +/// or directly; +/// the Encoding.Ascii-style static properties are fine, since .NET always has those. +/// +/// +public static class EncodingRegistry +{ + /// + /// Registers on first use. + /// + /// + /// is the point: registration must have + /// *completed* before any thread is allowed past, otherwise a second thread resolving concurrently + /// would call too early, catch the + /// and silently fall back — the exact bug this class exists to + /// prevent. Files load under Task.Run, so concurrent first resolves do happen. + /// + private static readonly Lazy _provider = new( + () => + { + Encoding.RegisterProvider(CodePagesEncodingProvider.Instance); + return true; + }, + LazyThreadSafetyMode.ExecutionAndPublication); + + private static void EnsureRegistered () + { + _ = _provider.Value; + } + + /// + /// Resolves a code page number. + /// + /// The code page number, e.g. 1252. + /// The for . + /// + /// is not a supported code page. Intended for hard-coded code pages, + /// where an unsupported value is a programming error rather than bad user input; use + /// for values that come from a file. + /// + public static Encoding GetEncoding (int codePage) + { + EnsureRegistered(); + return Encoding.GetEncoding(codePage); + } + + /// + /// Resolves an encoding name, falling back when it cannot be resolved. + /// + /// An encoding name such as "windows-1252", possibly null or empty. + /// The encoding to return when is unusable. + /// The resolved encoding, or . + public static Encoding GetEncoding (string? name, Encoding fallback) + { + return TryGetEncoding(name, out var encoding) ? encoding : fallback; + } + + /// + /// Attempts to resolve an encoding name. + /// + /// An encoding name such as "windows-1252", possibly null or empty. + /// The resolved encoding, or null when the name is unusable. + /// + /// true when names a supported encoding; false when it is + /// null, blank or unknown. + /// + public static bool TryGetEncoding (string? name, [NotNullWhen(true)] out Encoding? encoding) + { + encoding = null; + + if (string.IsNullOrWhiteSpace(name)) + { + return false; + } + + EnsureRegistered(); + + try + { + encoding = Encoding.GetEncoding(name); + return true; + } + catch (ArgumentException) + { + return false; + } + catch (NotSupportedException) + { + // Thrown for code pages the provider knows of but cannot instantiate. + return false; + } + } +} diff --git a/src/LogExpert.Tests/ColumnizerTests/CSVColumnizerTest.cs b/src/LogExpert.Tests/ColumnizerTests/CSVColumnizerTest.cs index 5706eb4a9..0a2561b74 100644 --- a/src/LogExpert.Tests/ColumnizerTests/CSVColumnizerTest.cs +++ b/src/LogExpert.Tests/ColumnizerTests/CSVColumnizerTest.cs @@ -524,9 +524,8 @@ public void LogfileReader_CommaCsv_ReloadWithPreProcess_DataLineNotEmpty () } /// - /// Tests the exact GUI scenario: single file (not multi), which enables the MemoryMappedFileReader. - /// The MMF reader reads raw lines without PreProcess, which can conflict with the buffer system - /// where lines are dropped. + /// Tests the exact GUI scenario: a single file opened with multiFile false, with a PreProcess + /// columnizer that drops the header line. /// [Test] public void LogfileReader_CommaCsv_SingleFile_WithPreProcess_DataLineNotEmpty () @@ -537,7 +536,7 @@ public void LogfileReader_CommaCsv_SingleFile_WithPreProcess_DataLineNotEmpty () using ManualResetEventSlim loadingDone = new(false); - // multiFile=FALSE — this enables the MemoryMappedFileReader path (like the real GUI) + // multiFile=FALSE, like the real GUI LogfileReader reader = new(path, new EncodingOptions(), false, 40, 50, new MultiFileOptions(), ReaderType.System, PluginRegistry.PluginRegistry.Instance, 500); reader.PreProcessColumnizer = csvColumnizer; reader.LoadingFinished += (_, _) => loadingDone.Set(); diff --git a/src/LogExpert.Tests/Dialogs/SettingsDialogEncodingListTests.cs b/src/LogExpert.Tests/Dialogs/SettingsDialogEncodingListTests.cs new file mode 100644 index 000000000..8b90ab4d7 --- /dev/null +++ b/src/LogExpert.Tests/Dialogs/SettingsDialogEncodingListTests.cs @@ -0,0 +1,47 @@ +using System.Text; + +using LogExpert.Core.Helpers; +using LogExpert.Dialogs; + +using NUnit.Framework; + +namespace LogExpert.Tests.Dialogs; + +/// +/// The Preferences encoding dropdown is the only way to set Preferences.DefaultEncoding, so the +/// list is asserted directly — building the dialog is not needed to know what it offers. +/// +[TestFixture] +public class SettingsDialogEncodingListTests +{ + [Test] + [TestCase(1250, TestName = "GetAvailableEncodings_OffersWindows1250")] + [TestCase(1252, TestName = "GetAvailableEncodings_OffersWindows1252")] + public void GetAvailableEncodings_OffersLegacyCodePage (int codePage) + { + var encodings = SettingsDialog.GetAvailableEncodings(); + + Assert.That(encodings.Select(encoding => encoding.CodePage), Does.Contain(codePage)); + } + + /// + /// Every offered encoding is saved as its name and resolved from that name on the next start, so a + /// name that cannot be resolved again would silently degrade to . + /// + [Test] + public void GetAvailableEncodings_EveryEntryResolvesByItsPersistedName () + { + var encodings = SettingsDialog.GetAvailableEncodings(); + + Assert.Multiple(() => + { + foreach (var encoding in encodings) + { + Assert.That( + EncodingRegistry.TryGetEncoding(encoding.HeaderName, out _), + Is.True, + $"'{encoding.HeaderName}' cannot be resolved back from a saved preference"); + } + }); + } +} diff --git a/src/LogExpert.Tests/Encodings/EncodingJsonConverterTests.cs b/src/LogExpert.Tests/Encodings/EncodingJsonConverterTests.cs new file mode 100644 index 000000000..5db2cc6a5 --- /dev/null +++ b/src/LogExpert.Tests/Encodings/EncodingJsonConverterTests.cs @@ -0,0 +1,62 @@ +using System.Text; + +using LogExpert.Core.Classes.JsonConverters; +using LogExpert.Core.Helpers; + +using Newtonsoft.Json; + +using NUnit.Framework; + +namespace LogExpert.Tests.Encodings; + +/// +/// The converter used for every in the settings JSON. It runs during +/// ConfigManager initialisation — long before any dialog exists — so it must be able to resolve a +/// legacy Windows code page on its own. +/// +[TestFixture] +public class EncodingJsonConverterTests +{ + [Test] + [TestCase("windows-1250", 1250)] + [TestCase("windows-1252", 1252)] + public void ReadJson_LegacyCodePageName_ResolvesInsteadOfFallingBack (string encodingName, int expectedCodePage) + { + var encoding = Deserialize($"\"{encodingName}\""); + + Assert.Multiple(() => + { + Assert.That(encoding, Is.Not.Null); + Assert.That(encoding.CodePage, Is.EqualTo(expectedCodePage)); + }); + } + + [Test] + public void ReadJson_Null_ReturnsNull () + { + Assert.That(Deserialize("null"), Is.Null); + } + + [Test] + [TestCase("\"\"")] + [TestCase("\"not-a-real-encoding-xxxxx\"")] + public void ReadJson_UnusableName_ReturnsDefaultEncoding (string json) + { + Assert.That(Deserialize(json), Is.EqualTo(Encoding.Default)); + } + + [Test] + public void WriteJson_RoundTripsALegacyCodePage () + { + // Resolve through the registry, not Encoding.GetEncoding — otherwise this test would depend on + // some earlier test having registered the provider. + var written = JsonConvert.SerializeObject(EncodingRegistry.GetEncoding(1252), new EncodingJsonConverter()); + + Assert.That(Deserialize(written)?.CodePage, Is.EqualTo(1252)); + } + + private static Encoding? Deserialize (string json) + { + return JsonConvert.DeserializeObject(json, new EncodingJsonConverter()); + } +} diff --git a/src/LogExpert.Tests/Encodings/EncodingRegistryTests.cs b/src/LogExpert.Tests/Encodings/EncodingRegistryTests.cs new file mode 100644 index 000000000..0ff4fcef7 --- /dev/null +++ b/src/LogExpert.Tests/Encodings/EncodingRegistryTests.cs @@ -0,0 +1,60 @@ +using System.Text; + +using LogExpert.Core.Helpers; + +using NUnit.Framework; + +namespace LogExpert.Tests.Encodings; + +/// +/// The legacy Windows code pages used to become resolvable only as a side effect of constructing +/// the Preferences dialog. Anything that resolved an encoding name earlier — Preferences, a .lxp, +/// the settings JSON — silently fell back to . These tests pin the +/// guarantee that resolving goes through instead. +/// +[TestFixture] +public class EncodingRegistryTests +{ + [Test] + [TestCase(1250)] + [TestCase(1252)] + public void GetEncoding_LegacyCodePage_Resolves (int codePage) + { + var encoding = EncodingRegistry.GetEncoding(codePage); + + Assert.That(encoding.CodePage, Is.EqualTo(codePage)); + } + + [Test] + [TestCase("windows-1250")] + [TestCase("windows-1252")] + [TestCase("utf-8")] + [TestCase("iso-8859-1")] + public void TryGetEncoding_SupportedName_ReturnsTrueAndEncoding (string name) + { + var resolved = EncodingRegistry.TryGetEncoding(name, out var encoding); + + Assert.Multiple(() => + { + Assert.That(resolved, Is.True); + Assert.That(encoding.WebName, Is.EqualTo(name)); + }); + } + + [Test] + [TestCase(null)] + [TestCase("")] + [TestCase(" ")] + [TestCase("not-a-real-encoding-xxxxx")] + public void TryGetEncoding_UnusableName_ReturnsFalse (string? name) + { + var resolved = EncodingRegistry.TryGetEncoding(name, out var encoding); + + Assert.Multiple(() => + { + Assert.That(resolved, Is.False); + Assert.That(encoding, Is.Null); + }); + } + +} diff --git a/src/LogExpert.Tests/Encodings/PersisterXmlEncodingTests.cs b/src/LogExpert.Tests/Encodings/PersisterXmlEncodingTests.cs new file mode 100644 index 000000000..a2a1e0444 --- /dev/null +++ b/src/LogExpert.Tests/Encodings/PersisterXmlEncodingTests.cs @@ -0,0 +1,90 @@ +using System.Text; + +using LogExpert.Core.Classes.Persister; + +using NUnit.Framework; + +namespace LogExpert.Tests.Encodings; + +/// +/// The per-file encoding stored in a .lxp. It is read while a file is being opened, with no dialog +/// involved, so a legacy Windows code page has to resolve on its own here too — otherwise the encoding +/// the user chose for that specific file is silently replaced by . +/// +[TestFixture] +#pragma warning disable CS0618 // PersisterXML is the deprecated fallback format, and still loads old .lxp files. +public class PersisterXmlEncodingTests +{ + private string _lxpFile = null!; + + [SetUp] + public void Setup () + { + _lxpFile = Path.Join(Path.GetTempPath(), $"{Guid.NewGuid()}.lxp"); + } + + [TearDown] + public void Cleanup () + { + if (File.Exists(_lxpFile)) + { + File.Delete(_lxpFile); + } + } + + [Test] + [TestCase("windows-1250", 1250)] + [TestCase("windows-1252", 1252)] + [TestCase("utf-8", 65001)] + public void Load_PersistedEncoding_Resolves (string encodingName, int expectedCodePage) + { + WriteLxp($""); + + var data = PersisterXML.Load(_lxpFile); + + Assert.Multiple(() => + { + Assert.That(data.Encoding, Is.Not.Null, "the persisted encoding was discarded"); + Assert.That(data.Encoding.CodePage, Is.EqualTo(expectedCodePage)); + }); + } + + [Test] + public void Load_NoEncodingElement_LeavesEncodingNull () + { + WriteLxp(string.Empty); + + var data = PersisterXML.Load(_lxpFile); + + // Null means "nothing was persisted for this file", which lets the BOM and the Preferences + // default decide. It must not be conflated with an unresolvable name. + Assert.That(data.Encoding, Is.Null); + } + + [Test] + public void Load_UnresolvableEncodingName_FallsBackToDefault () + { + WriteLxp(""); + + var data = PersisterXML.Load(_lxpFile); + + Assert.That(data.Encoding, Is.EqualTo(Encoding.Default)); + } + + private void WriteLxp (string encodingElement) + { + // has to be present: PersisterXML.ReadOptions dereferences it unconditionally. + File.WriteAllText( + _lxpFile, + $""" + + + + + {encodingElement} + + + """); + } +} +#pragma warning restore CS0618 diff --git a/src/LogExpert.Tests/Services/FileOperationServiceTests.cs b/src/LogExpert.Tests/Services/FileOperationServiceTests.cs index 52b9a07f6..f00aa7d85 100644 --- a/src/LogExpert.Tests/Services/FileOperationServiceTests.cs +++ b/src/LogExpert.Tests/Services/FileOperationServiceTests.cs @@ -303,6 +303,33 @@ public void AddFileTab_ValidDefaultEncoding_SetsDefaultEncoding () Assert.That(_factoryCalls[0].Encoding.DefaultEncoding.WebName, Is.EqualTo("utf-8")); } + /// + /// The headline bug: .NET does not ship the legacy Windows code pages, and registration of + /// CodePagesEncodingProvider used to happen only in the Preferences dialog constructor. A user + /// who picked Windows-1252 and restarted without reopening Preferences got their choice silently + /// discarded here, because this method swallows the resolve failure and leaves DefaultEncoding null. + /// + [Test] + [TestCase("windows-1250", 1250)] + [TestCase("windows-1252", 1252)] + public void AddFileTab_LegacyCodePageDefaultEncoding_ResolvesWithoutOpeningPreferences (string encodingName, int expectedCodePage) + { + // Arrange + _settings.Preferences.DefaultEncoding = encodingName; + _ = _tabControllerMock + .Setup(tc => tc.FindWindowByFileName(It.IsAny())) + .Returns((LogWindow)null!); + + var request = new FileTabRequest { FileName = "test.log" }; + + // Act + _ = _sut.AddFileTab(request); + + // Assert + Assert.That(_factoryCalls[0].Encoding.DefaultEncoding, Is.Not.Null, "the configured code page was discarded"); + Assert.That(_factoryCalls[0].Encoding.DefaultEncoding.CodePage, Is.EqualTo(expectedCodePage)); + } + [Test] public void AddFileTab_InvalidDefaultEncoding_DefaultEncodingRemainsNull () { diff --git a/src/LogExpert.Tests/StreamReaderTests/LogfileReaderEncodingTests.cs b/src/LogExpert.Tests/StreamReaderTests/LogfileReaderEncodingTests.cs new file mode 100644 index 000000000..63b19ab29 --- /dev/null +++ b/src/LogExpert.Tests/StreamReaderTests/LogfileReaderEncodingTests.cs @@ -0,0 +1,135 @@ +using System.Text; + +using LogExpert.Core.Classes.Log; +using LogExpert.Core.Classes.Log.ProgressReporters; +using LogExpert.Core.Entities; +using LogExpert.Core.Enums; +using LogExpert.Core.Helpers; + +using NUnit.Framework; + +namespace LogExpert.Tests.StreamReaderTests; + +/// +/// Pins the encoding a ends up using — the value that reaches the grid, +/// the Encoding menu and the persisted .lxp. +/// +[TestFixture] +public class LogfileReaderEncodingTests +{ + private const string EURO_LINE = "Euro: €"; + + private string _tempFile = null!; + + [SetUp] + public void Setup () + { + _tempFile = Path.GetTempFileName(); + _ = PluginRegistry.PluginRegistry.Create(Path.GetDirectoryName(_tempFile)!, 500); + } + + [TearDown] + public void Cleanup () + { + if (File.Exists(_tempFile)) + { + File.Delete(_tempFile); + } + } + + [Test] + public void ReadFiles_BomlessFile_UsesConfiguredDefaultEncoding () + { + var configuredEncoding = EncodingRegistry.GetEncoding(1252); + File.WriteAllText(_tempFile, EURO_LINE + "\n", configuredEncoding); + + using var reader = CreateReader(new EncodingOptions { DefaultEncoding = configuredEncoding }); + reader.ReadFiles(); + + Assert.Multiple(() => + { + Assert.That(reader.CurrentEncoding.CodePage, Is.EqualTo(configuredEncoding.CodePage)); + Assert.That(LineText(reader, 0), Is.EqualTo(EURO_LINE)); + }); + } + + [Test] + public void ReadFiles_PreamblePresent_OverridesConfiguredDefaultEncoding () + { + File.WriteAllText(_tempFile, EURO_LINE + "\n", new UTF8Encoding(encoderShouldEmitUTF8Identifier: true)); + + using var reader = CreateReader(new EncodingOptions { DefaultEncoding = EncodingRegistry.GetEncoding(1252) }); + reader.ReadFiles(); + + Assert.Multiple(() => + { + Assert.That(reader.CurrentEncoding.WebName, Is.EqualTo(Encoding.UTF8.WebName)); + Assert.That(LineText(reader, 0), Is.EqualTo(EURO_LINE)); + }); + } + + /// + /// Deliberate precedence: an explicit is either a choice + /// from the Encoding menu or one persisted per file in the .lxp, so it outranks the file's BOM. + /// Only (the Preferences value) yields to a BOM. + /// + [Test] + public void ReadFiles_ExplicitEncoding_TakesPrecedenceOverPreamble () + { + var explicitEncoding = EncodingRegistry.GetEncoding(1252); + File.WriteAllText(_tempFile, EURO_LINE + "\n", new UTF8Encoding(encoderShouldEmitUTF8Identifier: true)); + + using var reader = CreateReader(new EncodingOptions { Encoding = explicitEncoding }); + reader.ReadFiles(); + + Assert.That(reader.CurrentEncoding.CodePage, Is.EqualTo(explicitEncoding.CodePage)); + } + + /// + /// The last link of the chain: nothing explicit, no BOM, no Preferences default — the machine + /// default is what remains. + /// + [Test] + public void ReadFiles_NoExplicitEncodingNoPreambleNoConfiguredDefault_UsesTheMachineDefault () + { + File.WriteAllText(_tempFile, "plain ascii\n", Encoding.ASCII); + + using var reader = CreateReader(new EncodingOptions()); + reader.ReadFiles(); + + Assert.That(reader.CurrentEncoding.CodePage, Is.EqualTo(Encoding.Default.CodePage)); + } + + [Test] + public void ChangeEncoding_SwitchesTheReportedEncoding () + { + File.WriteAllText(_tempFile, "plain ascii\n", Encoding.ASCII); + + using var reader = CreateReader(new EncodingOptions { Encoding = Encoding.ASCII }); + reader.ReadFiles(); + + reader.ChangeEncoding(Encoding.Latin1); + + Assert.That(reader.CurrentEncoding.CodePage, Is.EqualTo(Encoding.Latin1.CodePage)); + } + + private static string? LineText (LogfileReader reader, int lineNum) + { + return reader.GetLogLineMemory(lineNum)?.FullLine.Span.ToString(); + } + + private LogfileReader CreateReader (EncodingOptions encodingOptions) + { + return new LogfileReader( + _tempFile, + encodingOptions, + multiFile: false, + bufferCount: 100, + linesPerBuffer: 500, + new MultiFileOptions(), + ReaderType.SystemDirect, + PluginRegistry.PluginRegistry.Instance, + maximumLineLength: 500, + progressReporter: NullProgressReporter.Instance); + } +} diff --git a/src/LogExpert.UI/Dialogs/SettingsDialog.cs b/src/LogExpert.UI/Dialogs/SettingsDialog.cs index 8207efdfb..de9804533 100644 --- a/src/LogExpert.UI/Dialogs/SettingsDialog.cs +++ b/src/LogExpert.UI/Dialogs/SettingsDialog.cs @@ -9,6 +9,7 @@ using LogExpert.Core.Config; using LogExpert.Core.Entities; using LogExpert.Core.Enums; +using LogExpert.Core.Helpers; using LogExpert.Core.Interfaces; using LogExpert.UI.ControlCharDisplay; using LogExpert.UI.Controls.LogTabWindow; @@ -86,8 +87,6 @@ private SettingsDialog (Preferences prefs, LogTabWindow logTabWin) LoadResources(); - Encoding.RegisterProvider(CodePagesEncodingProvider.Instance); - ResumeLayout(); } @@ -281,7 +280,7 @@ private void FillDialog () FillReaderTypeList(); FillControlCharsTab(); - comboBoxEncoding.SelectedItem = Encoding.GetEncoding(Preferences.DefaultEncoding); + comboBoxEncoding.SelectedItem = EncodingRegistry.GetEncoding(Preferences.DefaultEncoding, Encoding.Default); comboBoxLanguage.SelectedItem = CultureInfo.GetCultureInfo(Preferences.DefaultLanguage).Name; switch (Preferences.ColumnizerSelectionPriority) @@ -689,27 +688,44 @@ private void DisplayCurrentIcon () } /// - /// Populates the encoding list in the combo box with a predefined set of character encodings. + /// Populates the encoding list in the combo box from . The value + /// member of the combo box is set to a specific header name defined in the resources. /// - /// - /// This method clears any existing items in the combo box and adds a selection of common encodings, including - /// ASCII, Default (UTF-8), ISO-8859-1, UTF-8, Unicode, and Windows-1252. The value member of the combo box is set - /// to a specific header name defined in the resources. - /// private void FillEncodingList () { comboBoxEncoding.Items.Clear(); - _ = comboBoxEncoding.Items.Add(Encoding.ASCII); - _ = comboBoxEncoding.Items.Add(Encoding.Default); - _ = comboBoxEncoding.Items.Add(Encoding.GetEncoding("iso-8859-1")); - _ = comboBoxEncoding.Items.Add(Encoding.UTF8); - _ = comboBoxEncoding.Items.Add(Encoding.Unicode); - _ = comboBoxEncoding.Items.Add(CodePagesEncodingProvider.Instance.GetEncoding(1252)); + foreach (var encoding in GetAvailableEncodings()) + { + _ = comboBoxEncoding.Items.Add(encoding); + } comboBoxEncoding.ValueMember = Resources.SettingsDialog_UI_ComboBox_Encoding_ValueMember_HeaderName; } + /// + /// The encodings offered as the default encoding: ASCII, Default (UTF-8), ISO-8859-1, UTF-8, + /// Unicode, Windows-1250 and Windows-1252. + /// + /// + /// Separate from so the offered set can be asserted without building + /// the dialog. The selected entry is saved by name, so every entry has to be resolvable by name on + /// the next start — which is why the code pages go through . + /// + internal static IReadOnlyList GetAvailableEncodings () + { + return + [ + Encoding.ASCII, + Encoding.Default, + Encoding.Latin1, + Encoding.UTF8, + Encoding.Unicode, + EncodingRegistry.GetEncoding(1250), + EncodingRegistry.GetEncoding(1252) + ]; + } + /// /// Populates the language selection list with available language options. /// diff --git a/src/LogExpert.UI/Services/FileOperationService/FileOperationService.cs b/src/LogExpert.UI/Services/FileOperationService/FileOperationService.cs index a8a77d51b..3f5d511b0 100644 --- a/src/LogExpert.UI/Services/FileOperationService/FileOperationService.cs +++ b/src/LogExpert.UI/Services/FileOperationService/FileOperationService.cs @@ -6,6 +6,7 @@ using LogExpert.Core.Classes.Filter; using LogExpert.Core.Classes.Persister; using LogExpert.Core.Entities; +using LogExpert.Core.Helpers; using LogExpert.Core.Interfaces; using LogExpert.UI.Controls.LogWindow; using LogExpert.UI.Interface; @@ -120,17 +121,20 @@ public void AddToFileHistory (string fileName) private void FillDefaultEncodingFromSettings (EncodingOptions encodingOptions) { - if (_configManager.Settings.Preferences.DefaultEncoding != null) + var configuredEncoding = _configManager.Settings.Preferences.DefaultEncoding; + if (configuredEncoding == null) { - try - { - encodingOptions.DefaultEncoding = Encoding.GetEncoding(_configManager.Settings.Preferences.DefaultEncoding); - } - catch (ArgumentException) - { - _logger.Warn($"### FillDefaultEncodingFromSettings: Encoding {_configManager.Settings.Preferences.DefaultEncoding} is not a valid encoding"); - encodingOptions.DefaultEncoding = null; - } + return; + } + + if (EncodingRegistry.TryGetEncoding(configuredEncoding, out var encoding)) + { + encodingOptions.DefaultEncoding = encoding; + } + else + { + _logger.Warn($"### FillDefaultEncodingFromSettings: Encoding {configuredEncoding} is not a valid encoding"); + encodingOptions.DefaultEncoding = null; } }