You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Category: bug Area: log reading / multi-file Carved out of:#444 (which keeps the design questions — see Out of scope) Found: while removing the dead memory-mapped reader on branch fix/configured-encoding-registry
What happens
Open app.log in a directory that also contains app.log.1. Both files are loaded into the tab as one
logical stream, even though multi-file mode is off — File → Multi-file is unchecked and nothing in the
UI indicates that a second file was pulled in.
Cause
LogfileReader's single private constructor does:
IsMultiFile=multiFile||fileNames.Length==1;
Both public constructors funnel into it. The single-file overload passes [fileName], so fileNames.Length == 1 and IsMultiFile becomes true. The multi-file overload hard-codes multiFile: true. IsMultiFile is therefore true in every reachable configuration and the multiFile parameter never changes the outcome.
Because it is true, the constructor resolves names through new RolloverFilenameHandler(GetLogFileInfo(_fileName), _multiFileOptions).GetNameList(...) instead of [_fileName]. The default MultiFileOptions.FormatPattern is *$J(.) — an index pattern — so the handler
probes app.log.1, app.log.2, … on disk and adds every one that exists. _fileName is then reassigned to fileInfo.FullName of the last file found.
Note that LogWindow.IsMultiFile (the window flag, the one written to persistence) is a separate field
that correctly defaults to false. So the window says "single file" while its reader says "multi file".
Persisted .lxp sessions are not polluted by this and need no migration.
Reproduce
Create app.log with 10 lines and app.log.1 with 10 lines in the same directory.
Open onlyapp.log in LogExpert.
The tab shows 20 lines. File → Multi-file is unchecked.
The fix
IsMultiFile=multiFile;
Multi-file is an explicit caller decision and is never inferred from the array length. In particular the
multi-file overload keeps hard-coding true: RolloverFilenameHandlerexpands one name into many, so
"one filename + a rollover pattern" is a legitimate multi-file case, not a contradiction. #444's framing
that "it's only multifile if there are 2 or more files in the array" does not survive contact with the code
— BufferShiftTest constructs the reader with a single filename and multiFile: true, and that is exactly
the rollover scenario the feature exists for.
Blast radius — three dead branches come back to life
Because IsMultiFile has been unconditionally true, three code paths have never executed in any recent
release. The fix turns all three on for single-file tabs. This is the part that needs care in review:
names = [_fileName] in the constructor. Trivially correct; this is the fix's whole point.
The else branch of FireChangeEvent. Today, when a watched file shrinks or is deleted
(if (newSize < FileSize || _isDeleted)), a single-file tab runs ShiftBuffers() and reports IsRollover = true. After the fix it calls _progressReporter.ReportNewFile(...).
LogWindow's NewFile handling. ReportNewFile is the only producer of LoadFileEventArgs.NewFile,
so LogWindow's if (e.NewFile) → ReloadNewFile() is dead today too. ReloadNewFile does SavePersistenceData → full LoadFile → ClearBookmarkList() → SavePersistenceData, and ClearBookmarkList calls _bookmarkProvider.ClearAllBookmarks() — manual bookmarks included, then
persists the empty list.
Accepted consequence: truncating or deleting a tailed single-file log now reloads the tab and clears all
its bookmarks. That is the intended single-file semantic — the content those bookmarks pointed at is gone.
Deletion and in-place truncation (newSize < FileSize, e.g. a writer that reopens the same file with
truncate) are deliberately treated the same way; the line numbers are equally meaningless in both cases. ReloadNewFile itself is not to be modified under this issue.
FileOperationService.LoadFilesWithOption's fileNames.Length == 1 short-circuit, which means a single
file can never enter multi-file mode from drag/drop even with Shift held. That is Multi File Behaviour #444's "opening a file
from the open menu should have a multi-file option".
Moving the flag out of the constructor. Multi File Behaviour #444 says it "should be removed from the constructor … it has to
find its way into the class from the UI". Keep both public constructors and the bool multiFile
parameter exactly as they are.
SwitchMultiFile semantics on an open tab. This fix makes File → Multi-file off do something for the
first time (today toggling off still re-expands rollovers); refining what it should do — reactive vs
observable — is Multi File Behaviour #444's.
ReloadNewFile / bookmark clearing, per above.
Wiki updates.
Acceptance criteria
Tests 1–3 go in a new file src/LogExpert.Tests/StreamReaderTests/LogfileReaderMultiFileFlagTests.cs, each
building a temp directory containing both app.log and app.log.1 so the default *$J(.) pattern has
something to find. GetLogFileInfoList() is public and is the assertion surface:
SingleFileCtor_MultiFileFalse_DoesNotExpandRollover — multiFile: false ⇒ IsMultiFile == false, GetLogFileInfoList() has exactly one entry, LineCount equals only app.log's line count. This test fails before the fix.
SingleFileCtor_MultiFileTrue_ExpandsRollover — multiFile: true ⇒ IsMultiFile == true, two entries.
Passes today and must keep passing; this is the "one name + pattern" feature guard.
MultiFileCtor_AlwaysMultiFile — the array overload with a single-element array ⇒ IsMultiFile == true. Pins the decision so nobody later adds && fileNames.Length > 1.
SingleFileReader_OnTruncation_ReportsNewFileNotRollover — placed in src/LogExpert.Tests/Buffers/ beside BufferShiftTest, not in the new file: it is a monitoring test,
and BufferShiftTest already solves the watch-and-mutate timing problem, so reuse its helpers. Start
monitoring a single-file reader with multiFile: false, truncate the file, assert a NewFile report
arrives and no IsRollover event does. Use a fake ILoadProgressReporter. Give it a generous timeout so
a flake is obvious rather than mysterious.
BufferShiftTest passes unmodified. Do not edit it. It is the proof that rollover still works.
Verification
dotnet test src/LogExpert.Tests/LogExpert.Tests.csproj
dotnet test src/LogExpert.Tests/LogExpert.Tests.csproj --filter BufferShiftTest
Optional manual check with src/tools/LogRotator/ while watching the files in LogExpert.
Definition of done
Branch fix/single-file-multifile-flag off Development, conventional-commit subject, no Co-Authored-By
trailer. Do not open the PR — commit, run the suite, and leave a drafted PR title/body for the
maintainer to post.
Files: src/LogExpert.Core/Classes/Log/LogfileReader.cs (private constructor, line ~153) plus the two test
files above. No production file other than LogfileReader.cs should change.
Category: bug
Area: log reading / multi-file
Carved out of: #444 (which keeps the design questions — see Out of scope)
Found: while removing the dead memory-mapped reader on branch
fix/configured-encoding-registryWhat happens
Open
app.login a directory that also containsapp.log.1. Both files are loaded into the tab as onelogical stream, even though multi-file mode is off — File → Multi-file is unchecked and nothing in the
UI indicates that a second file was pulled in.
Cause
LogfileReader's single private constructor does:Both public constructors funnel into it. The single-file overload passes
[fileName], sofileNames.Length == 1andIsMultiFilebecomestrue. The multi-file overload hard-codesmultiFile: true.IsMultiFileis thereforetruein every reachable configuration and themultiFileparameter never changes the outcome.Because it is
true, the constructor resolves names throughnew RolloverFilenameHandler(GetLogFileInfo(_fileName), _multiFileOptions).GetNameList(...)instead of[_fileName]. The defaultMultiFileOptions.FormatPatternis*$J(.)— an index pattern — so the handlerprobes
app.log.1,app.log.2, … on disk and adds every one that exists._fileNameis then reassigned tofileInfo.FullNameof the last file found.Note that
LogWindow.IsMultiFile(the window flag, the one written to persistence) is a separate fieldthat correctly defaults to
false. So the window says "single file" while its reader says "multi file".Persisted
.lxpsessions are not polluted by this and need no migration.Reproduce
app.logwith 10 lines andapp.log.1with 10 lines in the same directory.app.login LogExpert.The fix
Multi-file is an explicit caller decision and is never inferred from the array length. In particular the
multi-file overload keeps hard-coding
true:RolloverFilenameHandlerexpands one name into many, so"one filename + a rollover pattern" is a legitimate multi-file case, not a contradiction. #444's framing
that "it's only multifile if there are 2 or more files in the array" does not survive contact with the code
—
BufferShiftTestconstructs the reader with a single filename andmultiFile: true, and that is exactlythe rollover scenario the feature exists for.
Blast radius — three dead branches come back to life
Because
IsMultiFilehas been unconditionallytrue, three code paths have never executed in any recentrelease. The fix turns all three on for single-file tabs. This is the part that needs care in review:
names = [_fileName]in the constructor. Trivially correct; this is the fix's whole point.elsebranch ofFireChangeEvent. Today, when a watched file shrinks or is deleted(
if (newSize < FileSize || _isDeleted)), a single-file tab runsShiftBuffers()and reportsIsRollover = true. After the fix it calls_progressReporter.ReportNewFile(...).LogWindow'sNewFilehandling.ReportNewFileis the only producer ofLoadFileEventArgs.NewFile,so
LogWindow'sif (e.NewFile) → ReloadNewFile()is dead today too.ReloadNewFiledoesSavePersistenceData→ fullLoadFile→ClearBookmarkList()→SavePersistenceData, andClearBookmarkListcalls_bookmarkProvider.ClearAllBookmarks()— manual bookmarks included, thenpersists the empty list.
Accepted consequence: truncating or deleting a tailed single-file log now reloads the tab and clears all
its bookmarks. That is the intended single-file semantic — the content those bookmarks pointed at is gone.
Deletion and in-place truncation (
newSize < FileSize, e.g. a writer that reopens the same file withtruncate) are deliberately treated the same way; the line numbers are equally meaningless in both cases.
ReloadNewFileitself is not to be modified under this issue.Out of scope — all of this stays with #444
FileOperationService.LoadFilesWithOption'sfileNames.Length == 1short-circuit, which means a singlefile can never enter multi-file mode from drag/drop even with Shift held. That is Multi File Behaviour #444's "opening a file
from the open menu should have a multi-file option".
find its way into the class from the UI". Keep both public constructors and the
bool multiFileparameter exactly as they are.
SwitchMultiFilesemantics on an open tab. This fix makes File → Multi-file off do something for thefirst time (today toggling off still re-expands rollovers); refining what it should do — reactive vs
observable — is Multi File Behaviour #444's.
ReloadNewFile/ bookmark clearing, per above.Acceptance criteria
Tests 1–3 go in a new file
src/LogExpert.Tests/StreamReaderTests/LogfileReaderMultiFileFlagTests.cs, eachbuilding a temp directory containing both
app.logandapp.log.1so the default*$J(.)pattern hassomething to find.
GetLogFileInfoList()is public and is the assertion surface:SingleFileCtor_MultiFileFalse_DoesNotExpandRollover—multiFile: false⇒IsMultiFile == false,GetLogFileInfoList()has exactly one entry,LineCountequals onlyapp.log's line count.This test fails before the fix.
SingleFileCtor_MultiFileTrue_ExpandsRollover—multiFile: true⇒IsMultiFile == true, two entries.Passes today and must keep passing; this is the "one name + pattern" feature guard.
MultiFileCtor_AlwaysMultiFile— the array overload with a single-element array ⇒IsMultiFile == true. Pins the decision so nobody later adds&& fileNames.Length > 1.SingleFileReader_OnTruncation_ReportsNewFileNotRollover— placed insrc/LogExpert.Tests/Buffers/besideBufferShiftTest, not in the new file: it is a monitoring test,and
BufferShiftTestalready solves the watch-and-mutate timing problem, so reuse its helpers. Startmonitoring a single-file reader with
multiFile: false, truncate the file, assert aNewFilereportarrives and no
IsRolloverevent does. Use a fakeILoadProgressReporter. Give it a generous timeout soa flake is obvious rather than mysterious.
BufferShiftTestpasses unmodified. Do not edit it. It is the proof that rollover still works.Verification
Optional manual check with
src/tools/LogRotator/while watching the files in LogExpert.Definition of done
Branch
fix/single-file-multifile-flagoffDevelopment, conventional-commit subject, noCo-Authored-Bytrailer. Do not open the PR — commit, run the suite, and leave a drafted PR title/body for the
maintainer to post.
Files:
src/LogExpert.Core/Classes/Log/LogfileReader.cs(private constructor, line ~153) plus the two testfiles above. No production file other than
LogfileReader.csshould change.