Fix configured encoding fallback and add Windows-1250 option (2)

[Pr0metheus2]

Attempt nr.2 (from #671)
I am trying to rewrite changes from previous pull request #671. When tested these changes, the Logexpert was able to correctly save encoding stored in app setting into .lxp file of newly opened non-BOM file (encoded in Windows-1252).
When UTF-8 BOM file was opened (also first time), correct utf-8 encoding was saved regardless app setting, which is requested behaviour.

I am not saying this fix is perfect, but I was able to get final result to requirement, at least I think so :). This project is so complex and its hard to look/search everywhere, at least for me. If you are convinced this fix is not correct, please do it your way. I am tust trying to make this app better.

[Hirogen]

I am not saying this fix is perfect, but I was able to get final result to requirement, at least I think so :). This project is so complex and its hard to look/search everywhere, at least for me. If you are convinced this fix is not correct, please do it your way. I am tust trying to make this app better.

First and foremost thank you for participating and trying to help with improving LogExpert. I'm happy for every help regardless how big or small the changes are. I know from personal experience that it can take quite some time to understand the codebase and even though I refactored a lot of code in the last few months and tried to make it more readable and testable it has still a long way to go. This is also the reason I try to be precise with the review, please by all means don't take it personally, I just want to be thorough. I'm quite picky with changes in LogfileReader.cs and PositionAwareReaderxxx.cs because those are the core of the program and changes there can have quite the consequences (I learned that the hard way)

The Review:

1. [Program.cs]— LF lines injected into a CRLF file. every added line ends $ (LF) while all context ends ^M$, and five pre-existing lines were rewritten CRLF→LF:

• [SupportedOSPlatform("windows")]^M$ → + [SupportedOSPlatform("windows")]$
• private static void SetDarkMode ()^M$ → + private static void SetDarkMode ()$
  Breaches [src/.editorconfig] end_of_line = crlf — the SetDarkMode hunk is pure line-ending churn tracing to nothing in the request. [.gitattributes] pins only usedComponents.json, so nothing normalizes this away.

2. Final-newline churn in Program.cs and SettingsDialog
3. Hand-committed plugin hashes. ae5d3c2 + 7d17376 ("Update PluginHashGenerator.Generated.cs", 16 lines each, the second redoing the first) rewrite 16 unrelated plugin hashes from a local Release build. Hash should update via GitHub Actions on successful builds; Development records these as chore: update plugin hashes [skip ci]. (Since the Release Build still fails, this is something I need to update in the ci/cd)
4. The DetermineEncoding hunk is behavior-neutral. options?.Encoding != null ? options.Encoding : X → options?.Encoding ?? X is exactly equivalent; only the comment is new. Separately, the commit body claims the new order is "BOM → persisted/explicit" while code and the new comment do the opposite
5. Precedence is unchanged — the stated fix is a no-op. Spec: "Encoding precedence allowed an explicit/persisted value to override a file BOM" / "BOM → persisted/explicit encoding → Preferences.DefaultEncoding → Encoding.Default". Old: options?.Encoding != null ? options.Encoding : detectedEncoding ?? options?.DefaultEncoding ?? Encoding.Default. New: options?.Encoding ?? detectedEncoding ?? …. Semantically identical — explicit still beats BOM, and ReadFiles_ExplicitEncoding_OverridesBom now locks that in as intended behaviour. Fix configured encoding fallback and add Windows-1250 option (2) #673's claim ("utf-8 saved regardless app setting") holds only because EncodingOptions.Encoding is null on first open — a .lxp that already persisted windows-1252 will still override a later-added BOM.
6. No test for the headline .lxp claim, and the MMF BOM-skip is untested. The chain does work: Program.RegisterEncodingProvider() runs before ConfigManager.Initialize, so [FileOperationService.cs:121] FillDefaultEncodingFromSettings no longer throws on windows-1252 → DetermineEncoding → CurrentEncoding ([LogfileReader.cs:1309] → snapshot.Encoding ([LogWindow.cs:5921]. But nothing tests it. The new tests never reach the MMF reader: BuildIndex is only called from ExtendIndex/FileChanged ([LogfileReader.cs:1572], never from ReadFiles, so _mmfReader.LineCount == 0 and GetLogLineMemory(0) falls through to the buffer path. _lineIndex.Add(_preambleLength) has zero coverage.
7. "Both readers apply the configured default consistently" — only at load. MMF encoding and preambleLength are frozen in the LogfileReader ctor; ChangeEncoding ([LogfileReader.cs:750] never refreshes _mmfReader, and LogWindow.ChangeEncoding skips Reload() when preamble length and IsSingleByte match — which is exactly the new windows-1250 ↔ windows-1252 case.

Summary

1. PluginHashGenerator.Generated.cs (32 lines): build artifact, out of scope; CI already regenerates it.
2. RegisterEncodingProvider_MakesWindows1252Available in OneInstanceIpcTests — unrelated fixture, asserts BCL behaviour not spec behaviour.
3. CRLF/indentation churn in Program.cs (SetDarkMode) and EOF-newline changes.

What really is a problem:

1. The new FileStream widens the failure surface. [LogfileReader.cs:192], now performs real I/O where the old primary-ctor call did none, so catch (IOException) went from dead code to load-bearing — but UnauthorizedAccessException (denied read ACL, path is a directory) is not an IOException. [LogWindow.cs:5729] only catches LogFileException, and LoadFile runs under Task.Run, so such files now fail silently instead of opening. Disposal ordering is fine (MMF opens later, in BuildIndex).
2. Encoding.GetEncoding(1250)/(1252) in GetAvailableEncodings() can throw. Registration now lives only in LogExpert/Program.Main and is internal to the exe, so LogExpert.UI can no longer self-protect; the old CodePagesEncodingProvider.Instance.GetEncoding(1252) could not throw. Safe today (FillEncodingList runs from Fill(), not the ctor, and Main registers first), but it makes "regardless of whether Settings was opened first" dependent on one entry point. Windows-1250 is genuinely selectable.

[Hirogen]

I made changes to the PluginHashTool, please rebase your branch with those changes, then the release build should pass

[Pr0metheus2]

Seems I cannot make proper fix to fullfil all checks/requests. Sorry, I give up.

[Hirogen]

Seems I cannot make proper fix to fullfil all checks/requests. Sorry, I give up.

dont give up, i created a #677 pr, this is based on your findings, the mmr is actually not reachable and was dead code because it only was reachable if IsMultiFile is false, but this is always true, this is another bug, but will be fixed in one of the next pr's, your Idea with the Encodings leads to the EncodingRegistry that lives in core and now can be used by everybody and does not need to be used in Program.cs

[Hirogen]

thx for all the suggestion and input, #677 is all you :)

[Pr0metheus2]

Thanks for making this fix alive!